Re: FIT signature security flay

[Sean Anderson]

Hi Lev,

On 10/14/24 13:15, Lev R. Oshvang wrote:
> [You don't often get email from levon...@gmail.com. Learn why this is 
> important at 
> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2faka.ms%2fLearnAboutSenderIdentification&umid=d3086c41-4950-4369-b35b-dfbad134e05e&auth=d807158c60b7d2502abde8a2fc01f40662980862-da2f573d80b6a54ab9f8699b0376fb4e503517ea
>  ]
> 
> Hi Sean,
> Thanks for replying.
> Of course, I have public key embedded into u-boot dtb.
> I see it with dtdiff utility clearly., that it has rsa components and
> mention required property, but there are no means  :
> -   signature {
> -
> -   key-dev_key {
> -   algo = "sha1,rsa2048";
> -   key-name-hint = "dev_key";
> -   required = "conf";
> -   rsa,exponent = <0x00 0x10001>;
> -   rsa,modulus = <0xca0facd8 0xc8a49486
> 0x9785b0a8 0xd8560eae 0xffaefc34 0xd958e8d9 0xba5b1623 0x197b8ae
> 0x216b0699 0xb5f048ab 0x1167f69a 0x3d02f44b 0xe1bdc8f3 0x533eaa9c
> 0xd2ce119d 0xf34d90e7 0x9f470b92 0xa672fc84 0x25ce9a70 0x1ba0422f
> 0xa92f1dc2 0x8a6026e9 0xc06c080 0x23d300b0 0xe1c325aa 0x9c229a84
> 0x40a59d7 0x3f59c482 0x7eb27b44 0x9e6d300 0xbd36a4c4 0x5cc65b1
> 0xb5708d8a 0xfc19f30 0x11ce5b3 0xed2c646b 0x77492129 0x4b0382ab
> 0xcf7ac83d 0x93ca0078 0x6f4db3f7 0xd9934ef1 0x2bdb929c 0x4e0726fd
> 0x56568874 0xf0950b02 0x1b2c51ae 0x94d685a4 0x6edd9044 0xb62bb692
> 0x3b131cc8 0xce8c1649 0x41726a8 0x34282ad7 0x7c978b86 0xf970b5cc
> 0xc0505052 0x392a0a39 0xf9b25e93 0x5f32ff98 0x38b8ceda 0xda3a2855
> 0x2bbcb269 0x61db7b91>;
> -   rsa,n0-inverse = <0xc1255a8f>;
> -   rsa,num-bits = <0x800>;
> -   rsa,r-squared = <0x50fdcf54 0x76c283ab
> 0x897a6a96 0x5011d310 0xc70c897 0x638fdc61 0xbde79c4c 0x5a66d6a5
> 0x7747e613 0xcac6b564 0x62456d6c 0x73d3f181 0xfd1d48ae 0xf8159021
> 0xa5c7cadf 0xf3ea3aee 0x3a801e43 0xc4d573cd 0x2c7e8dc 0x44030a5d
> 0xa679da1f 0xad11fad2 0x93fc1da4 0xb3ca4d43 0x30cb4202 0xab21f661
> 0x57041882 0xa63b5c94 0x89c38732 0x8f2b191 0xe9e4a99 0x8292fc7f
> 0x6e7cf63a 0x9eef2fab 0xa1414bfc 0xfdea67c0 0x713fe78d 0xaf176725
> 0x72168246 0x7ab0706d 0xac7f19b4 0x500118c8 0x5915e449 0xaf2cf688
> 0xb70d5fbb 0x8740ba88 0xc89fde6a 0x91931a8d 0x915b76b5 0x5dfcb2e9
> 0x7fe48d92 0xfec26649 0x541dd9c7 0x82c4957 0xb1a7b46 0x1b29c87d
> 0xbb76c881 0x8da006a5 0xeaacff4 0xf39c1d12 0x82cc7dfa 0xc8de4237
> 0xf03ee80d 0xb060a204>;
> -   };
> -   };
> -
> 
> 
> But u-boot control dtb does not impose any requirement that FIT image
> structure MUST have.
> I think this is a reason for behaviour I observe.
> It would be better to add FIT nodes structure to uboot dtb for the
> case like mine
> 
> I am attaching my  2 its files, one that do requires signature and
> second that requires only a hash
> Both flawlessly boot kernel.
> 
> There is also script I use for signage and CTRL_FDT file my-iuboot.dtb

I compared your images to some examples I have and they seem correct.

It may seem silly, but do you have CONFIG_FIT_SIGNATURE set?

Also: note that `iminfo` will only report if *image* (not conf) signatures
are correct. The conf signatures will be verified, but only when you boot
(or source) the image using a particular config.

--Sean

> On Mon, Oct 14, 2024 at 5:24 PM Sean Anderson  wrote:
>>
>> Hi Lev,
>>
>> On 10/14/24 04:42, Lev R. Oshvang wrote:
>> > Hi Sean,
>> >
>> > I am looking for help with Uboot FIT  signatures problem
>> >
>> >
>> >
>> > I  started to work with FIT image (u-boot 2024)  and managed to sign
>> > kernel and load this image with Uboot using 'required' property in
>> > signature as  :
>> >
>> > signature-1 {
>> >
>> > algo = "sha1,rsa2048";
>> >
>> > key-name-hint = "dev_key";
>> >
>> > sign-images="kernel";
>> >
>> > required="conf";
>> >
>> >  {
>> >
>> > Iminfo reports"
>> >
>> > ## Checking hash(es) for FIT Image at 0100 ...
>> >
>> >Hash(es) for Image 0 (kernel-1): sha256+ sha256,rsa2048:dev_key-
>> >
>> >
>> >
>> > To test the procedure, I generated another private key and signed
>> > another kernel with this new key on another Linux host.
>> >
>> >
>> >
>> >
>> >
>> > I expected bootm to fail, but it just happily loads this image!!!
>> >
>> > Even an image without a signature but with a valid hash is not
>> > rejected against my expectations.
>> >
>> > In this case iminfo report only hash is OK
>>
>> Did you embed the public key into your U-Boot devicetree with `mkimage -K` ?
>>
>> --Sean

Re: FIT signature security flay

[Sean Anderson]

Hi Lev,

On 10/14/24 04:42, Lev R. Oshvang wrote:
> Hi Sean,
> 
> I am looking for help with Uboot FIT  signatures problem
> 
> 
> 
> I  started to work with FIT image (u-boot 2024)  and managed to sign
> kernel and load this image with Uboot using 'required' property in
> signature as  :
> 
> signature-1 {
> 
> algo = "sha1,rsa2048";
> 
> key-name-hint = "dev_key";
> 
> sign-images="kernel";
> 
> required="conf";
> 
>  {
> 
> Iminfo reports"
> 
> ## Checking hash(es) for FIT Image at 0100 ...
> 
>Hash(es) for Image 0 (kernel-1): sha256+ sha256,rsa2048:dev_key-
> 
> 
> 
> To test the procedure, I generated another private key and signed
> another kernel with this new key on another Linux host.
> 
> 
> 
> 
> 
> I expected bootm to fail, but it just happily loads this image!!!
> 
> Even an image without a signature but with a valid hash is not
> rejected against my expectations.
> 
> In this case iminfo report only hash is OK

Did you embed the public key into your U-Boot devicetree with `mkimage -K` ?

--Sean

Re: [PATCH v2 3/4] dm: core: migrate debug() messages to use dm_warn

[Sean Anderson]

On 10/2/24 05:25, Quentin Schulz wrote:

Hi Alexander,

On 10/2/24 10:37 AM, Alexander Dahl wrote:

Hello Quentin,

sorry for being late to the party, but I just tested v2024.10-rc6 and
my console output looks like this now:

 ofnode_read_bool: bootph-all: true
 ofnode_read_u32_array: ranges: ofnode_read_bool: bootph-all: false
 ofnode_read_bool: bootph-some-ram: false
 ofnode_read_bool: bootph-pre-ram: false
 ofnode_read_bool: bootph-pre-sram: false
 ofnode_read_bool: u-boot,dm-pre-reloc: false
 ofnode_read_bool: u-boot,dm-pre-proper: false
 ofnode_read_bool: u-boot,dm-spl: false
 ofnode_read_bool: u-boot,dm-tpl: false
 ofnode_read_bool: u-boot,dm-vpl: false
 ofnode_read_bool: bootph-all: false
 ofnode_read_bool: bootph-some-ram: false
 ofnode_read_bool: bootph-pre-ram: false
 ofnode_read_bool: bootph-pre-sram: false
 ofnode_read_bool: u-boot,dm-pre-reloc: false
 ofnode_read_bool: u-boot,dm-pre-proper: false
 ofnode_read_bool: u-boot,dm-spl: false
 ofnode_read_bool: u-boot,dm-tpl: false
 ofnode_read_bool: u-boot,dm-vpl: false
 ofnode_read_bool: bootph-all: true
 ofnode_read_u32_array: ranges: ofnode_read_bool: bootph-all: true
 ofnode_read_u32_array: ranges: ofnode_read_bool: bootph-all: true
 ofnode_read_bool: bootph-all: true
 ofnode_read_bool: bootph-all: false
 …

This goes on for several screen pages, and clutters the usual output.
I first thought I messed up CONFIG_LOGLEVEL or CONFIG_LOG, but no.
All I had done was setting CONFIG_DM_WARN …

Am Tue, Jun 11, 2024 at 03:04:26PM +0200 schrieb Quentin Schulz:

From: Quentin Schulz 

Prior to that, seeing the debug() messages required to enable DM_DEBUG
which defines DEBUG (and then _DEBUG) which in turn makes failing
assert() calls reset U-Boot which isn't necessarily what is desired.

Instead, let's migrate to dm_warn which is using log_debug when unset or
log_warn when set.

While at it, reword the DM_DEBUG symbol in Kconfig to explain what it
now actually does.


CONFIG_DM_WARN currently reads like this:

 Enable this to see warnings related to driver model.

 Warnings may help with debugging, such as when expected devices do
 not bind correctly. If the option is disabled, dm_warn() is compiled
 out - it will do nothing when called.

Instead of just useful warnings, users get debug messages all over the
place now.  Is this actually intended behaviour?  Can this be fixed
before v2024.10 release please?



There are basically less than 3 working days left before v2024.10 is released 
and I'm inclined to say this is annoying rather than a bug, so I am not 
entirely sure this will 1) make it in time if we agree on a fix (needs to 
include review process in those 3 working days) 2) be acceptable for a late 
addition in the release cycle. Let's try though, maybe we can figure something 
out.


This is a bug. Printing this many messages will have a major impact on boot 
times.

A config going from "may print out a few more lines of extra info" to "firehose"
is very surprising to users (as evidenced by Alexander's email).

If you can't figure out which lines to disable, I recommend simply reverting the
patch.

--Sean

Re: [PATCH] spl: spl_load: fix comparison between negative error code and unsigned size

[Sean Anderson]

On 8/30/24 23:17, Daniel Palmer wrote:

read could be a negative error value but size in spl_image is unsigned
so when they are compared read is used as if it's a unsigned value
and if it's negative it'll most likely be bigger than size and the
result will be true and _spl_load() will return 0 to the caller.


Then cast spl_image->size to long.


This results in the caller to _spl_load() not seeing that an error happened
as it should and continuing as if the load was completed when it might
not have been.

Check if read is negative and return it's value if it is before comparing
against size in spl_image.

Signed-off-by: Daniel Palmer 
---
  include/spl_load.h | 4 
  1 file changed, 4 insertions(+)

diff --git a/include/spl_load.h b/include/spl_load.h
index 1c2b296c0a2c..7de834f402b8 100644
--- a/include/spl_load.h
+++ b/include/spl_load.h
@@ -83,6 +83,10 @@ static inline int _spl_load(struct spl_image_info *spl_image,
  
  	read = info->read(info, offset + image_offset, size,

  map_sysmem(spl_image->load_addr - overhead, size));
+
+   if (read < 0)
+   return read;
+
return read < spl_image->size ? -EIO : 0;
  }
  

Re: [PATCH v2] spl: ram: hide SPL_RAM_SUPPORT

[Sean Anderson]

On 9/10/24 09:00, Jerome Forissier wrote:

Make SPL_RAM_SUPPORT a hidden Kconfig symbol, automatically selected
by SPL_RAM_DEVICE or SPL_DFU. Avoids the situation where SPL_RAM_SUPPORT
may be enabled without the other two being enabled, which results in the
following build warning:

common/spl/spl_ram.c:19:14: warning: ‘spl_ram_load_read’ defined but not used 
[-Wunused-function]
19 | static ulong spl_ram_load_read(struct spl_load_info *load, ulong 
sector,
   |  ^

Signed-off-by: Jerome Forissier 
---
  common/spl/Kconfig | 8 ++--
  drivers/usb/gadget/Kconfig | 2 +-
  2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index c08ff064493..885a012461c 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -1238,15 +1238,11 @@ config SPL_POWER_DOMAIN
  the drivers in drivers/power/domain as part of a SPL build.
  
  config SPL_RAM_SUPPORT

-   bool "Support booting from RAM"
-   default y if MICROBLAZE || ARCH_SOCFPGA || ARCH_TEGRA || ARCH_ZYNQ
-   help
- Enable booting of an image in RAM. The image can be preloaded or
- it can be loaded by SPL directly into RAM (e.g. using USB).
+   bool
  
  config SPL_RAM_DEVICE

bool "Support booting from preloaded image in RAM"
-   depends on SPL_RAM_SUPPORT
+   select SPL_RAM_SUPPORT
default y if MICROBLAZE || ARCH_SOCFPGA || ARCH_TEGRA || ARCH_ZYNQ
help
  Enable booting of an image already loaded in RAM. The image has to
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 4621a6fd5e6..b1247a9eeae 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -323,7 +323,7 @@ config SPL_DFU
bool "Support DFU (Device Firmware Upgrade) in SPL"
select SPL_HASH
select SPL_DFU_NO_RESET
-   depends on SPL_RAM_SUPPORT
+   select SPL_RAM_SUPPORT
help
  This feature enables the DFU (Device Firmware Upgrade) in SPL with
  RAM memory device support. The ROM code will load and execute


Reviewed-by: Sean Anderson 

Re: [PATCH] spl: ram: hide SPL_RAM_SUPPORT

[Sean Anderson]

On 9/10/24 05:00, Jerome Forissier wrote:

Make SPL_RAM_SUPPORT a hidden Kconfig symbol, automatically selected
by SPL_RAM_DEVICE or SPL_DFU. Avoids the situation where SPL_RAM_SUPPORT
may be enabled without the other two being enabled, which results in the
following build warning:

common/spl/spl_ram.c:19:14: warning: ‘spl_ram_load_read’ defined but not used 
[-Wunused-function]
19 | static ulong spl_ram_load_read(struct spl_load_info *load, ulong 
sector,
   |  ^

Signed-off-by: Jerome Forissier 
---
  common/spl/Kconfig | 8 ++--
  1 file changed, 2 insertions(+), 6 deletions(-)

Replaces "spl: ram: fix build warning when neither RAM_DEVICE nor DFU
are enabled" [1].

[1] https://lists.denx.de/pipermail/u-boot/2024-September/564302.html

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index c08ff064493..885a012461c 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -1238,15 +1238,11 @@ config SPL_POWER_DOMAIN
  the drivers in drivers/power/domain as part of a SPL build.
  
  config SPL_RAM_SUPPORT

-   bool "Support booting from RAM"
-   default y if MICROBLAZE || ARCH_SOCFPGA || ARCH_TEGRA || ARCH_ZYNQ
-   help
- Enable booting of an image in RAM. The image can be preloaded or
- it can be loaded by SPL directly into RAM (e.g. using USB).
+   bool
  
  config SPL_RAM_DEVICE

bool "Support booting from preloaded image in RAM"
-   depends on SPL_RAM_SUPPORT
+   select SPL_RAM_SUPPORT
default y if MICROBLAZE || ARCH_SOCFPGA || ARCH_TEGRA || ARCH_ZYNQ
help
  Enable booting of an image already loaded in RAM. The image has to


I think SPL_DFU should also select SPL_RAM_SUPPORT

--Sean

Re: [PATCH] spl: ram: fix build warning when neither RAM_DEVICE nor DFU are enabled

[Sean Anderson]

On 9/9/24 09:22, Jerome Forissier wrote:

Fixes the following warning:

common/spl/spl_ram.c:19:14: warning: ‘spl_ram_load_read’ defined but not used 
[-Wunused-function]
19 | static ulong spl_ram_load_read(struct spl_load_info *load, ulong 
sector,
   |  ^


Does this config even make sense? What is the point of enabling SPL_RAM_SUPPORT 
without one of the above configs?

Maybe we should make those configs select RAM_SUPPORT instead.


Signed-off-by: Jerome Forissier 
---
  common/spl/spl_ram.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/common/spl/spl_ram.c b/common/spl/spl_ram.c
index 71b7a8374bb..1a5874c3949 100644
--- a/common/spl/spl_ram.c
+++ b/common/spl/spl_ram.c
@@ -16,6 +16,7 @@
  #include 
  #include 
  
+#if CONFIG_IS_ENABLED(RAM_DEVICE) || CONFIG_IS_ENABLED(DFU)

  static ulong spl_ram_load_read(struct spl_load_info *load, ulong sector,
   ulong count, void *buf)
  {
@@ -96,6 +97,8 @@ static int spl_ram_load_image(struct spl_image_info 
*spl_image,
  
  	return ret;

  }
+#endif
+
  #if CONFIG_IS_ENABLED(RAM_DEVICE)
  SPL_LOAD_IMAGE_METHOD("RAM", 0, BOOT_DEVICE_RAM, spl_ram_load_image);
  #endif

[PATCH v2 2/2] arm: zynqmp: Enable non-invasive CCI-400 PMU debug

[Sean Anderson]

Set NIDEN, enabling non-invasive debug for the CCI-400 PMU. Otherwise,
the PMU is effectively disabled.

Signed-off-by: Sean Anderson 
Reviewed-by: Michal Simek 
---

(no changes since v1)

 arch/arm/mach-zynqmp/include/mach/hardware.h | 3 +++
 board/xilinx/zynqmp/zynqmp.c | 4 
 2 files changed, 7 insertions(+)

diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
b/arch/arm/mach-zynqmp/include/mach/hardware.h
index f1514d6a869..51eab3509b8 100644
--- a/arch/arm/mach-zynqmp/include/mach/hardware.h
+++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
@@ -128,6 +128,9 @@ struct crfapb_regs {
 
 #define crfapb_base ((struct crfapb_regs *)ZYNQMP_CRF_APB_BASEADDR)
 
+#define ZYNQMP_CCI_REG_CCI_MISC_CTRL   0xFD5E0040
+#define ZYNQMP_CCI_REG_CCI_MISC_CTRL_NIDEN BIT(1)
+
 #define ZYNQMP_APU_BASEADDR0xFD5C
 
 struct apu_regs {
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 56e3b36ca99..20a675c010d 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -76,6 +76,10 @@ int __maybe_unused psu_uboot_init(void)
writel(0x04920492, ZYNQMP_IOU_SECURE_SLCR);
writel(0x00920492, ZYNQMP_IOU_SECURE_SLCR + 4);
 
+   /* Enable CCI PMU events */
+   writel(ZYNQMP_CCI_REG_CCI_MISC_CTRL_NIDEN,
+  ZYNQMP_CCI_REG_CCI_MISC_CTRL);
+
/* Delay is required for clocks to be propagated */
udelay(100);

-- 
2.35.1.1320.gc452695387.dirty

[PATCH v2 1/2] zynqmp: Disable secure access for boot devices

[Sean Anderson]

Boot devices (QSPI, MMC, NAND, and Ethernet) use secure access for DMA
by default. As this causes problems when using the SMMU [1], configure
them for normal access instead.

[1] https://support.xilinx.com/s/article/72164

Signed-off-by: Sean Anderson 
---

Changes in v2:
- Don't set reserved bits in AXI_RPRTCN, since QSPI doesn't use ARPROT

 arch/arm/mach-zynqmp/include/mach/hardware.h | 2 ++
 board/xilinx/zynqmp/zynqmp.c | 4 
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
b/arch/arm/mach-zynqmp/include/mach/hardware.h
index 8cb6494e52c..f1514d6a869 100644
--- a/arch/arm/mach-zynqmp/include/mach/hardware.h
+++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
@@ -63,6 +63,8 @@ struct crlapb_regs {
 
 #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
 
+#define ZYNQMP_IOU_SECURE_SLCR 0xFF24
+
 #define ZYNQMP_IOU_SCNTR_SECURE0xFF26
 #define ZYNQMP_IOU_SCNTR_COUNTER_CONTROL_REGISTER_EN   0x1
 #define ZYNQMP_IOU_SCNTR_COUNTER_CONTROL_REGISTER_HDBG 0x2
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index e6331c0e4d8..56e3b36ca99 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -72,6 +72,10 @@ int __maybe_unused psu_uboot_init(void)
writel(ZYNQMP_PS_SYSMON_ANALOG_BUS_VAL,
   ZYNQMP_AMS_PS_SYSMON_ANALOG_BUS);
 
+   /* Disable secure access for boot devices */
+   writel(0x04920492, ZYNQMP_IOU_SECURE_SLCR);
+   writel(0x00920492, ZYNQMP_IOU_SECURE_SLCR + 4);
+
/* Delay is required for clocks to be propagated */
udelay(100);

-- 
2.35.1.1320.gc452695387.dirty

[PATCH v2 0/2] arm: zynqmp: Initialize some registers at boot

[Sean Anderson]

These patches are independent in intent, but they modify adjacent
lines so I have sent them as a series.

Changes in v2:
- Don't set reserved bits in AXI_RPRTCN, since QSPI doesn't use ARPROT

Sean Anderson (2):
  zynqmp: Disable secure access for boot devices
  arm: zynqmp: Enable non-invasive CCI-400 PMU debug

 arch/arm/mach-zynqmp/include/mach/hardware.h | 5 +
 board/xilinx/zynqmp/zynqmp.c | 8 
 2 files changed, 13 insertions(+)

-- 
2.35.1.1320.gc452695387.dirty

Re: [PATCH 1/2] zynqmp: Disable secure access for boot devices

[Sean Anderson]

On 9/2/24 04:43, Michal Simek wrote:
> 
> 
> On 8/13/24 00:07, Sean Anderson wrote:
>>
>> Boot devices (QSPI, MMC, NAND, and Ethernet) use secure access for DMA
>> by default. As this causes problems when using the SMMU [1], configure
>> them for normal access instead.
>>
>> [1] https://support.xilinx.com/s/article/72164
>>
>> Signed-off-by: Sean Anderson 
>> ---
>>
>>   arch/arm/mach-zynqmp/include/mach/hardware.h | 2 ++
>>   board/xilinx/zynqmp/zynqmp.c | 4 
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
>> b/arch/arm/mach-zynqmp/include/mach/hardware.h
>> index 8cb6494e52c..f1514d6a869 100644
>> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
>> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
>> @@ -63,6 +63,8 @@ struct crlapb_regs {
>>
>>   #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
>>
>> +#define ZYNQMP_IOU_SECURE_SLCR 0xFF24
>> +
>>   #define ZYNQMP_IOU_SCNTR_SECURE    0xFF26
>>   #define ZYNQMP_IOU_SCNTR_COUNTER_CONTROL_REGISTER_EN   0x1
>>   #define ZYNQMP_IOU_SCNTR_COUNTER_CONTROL_REGISTER_HDBG 0x2
>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>> index b4c15b041cc..94a66684af2 100644
>> --- a/board/xilinx/zynqmp/zynqmp.c
>> +++ b/board/xilinx/zynqmp/zynqmp.c
>> @@ -72,6 +72,10 @@ int __maybe_unused psu_uboot_init(void)
>>  writel(ZYNQMP_PS_SYSMON_ANALOG_BUS_VAL,
>>     ZYNQMP_AMS_PS_SYSMON_ANALOG_BUS);
>>
>> +   /* Disable secure access for boot devices */
>> +   writel(0x04920492, ZYNQMP_IOU_SECURE_SLCR);
>> +   writel(0x04920492, ZYNQMP_IOU_SECURE_SLCR + 4);
> 
> Based on
> https://docs.amd.com/r/en-US/ug1087-zynq-ultrascale-registers/IOU_AXI_RPRTCN-IOU_SECURE_SLCR-Register
> 
> there is no definition for bits 27:25 that's why that upper 4 shouldn't be 
> there.

OK. Will fix for v2.

--Sean

Re: Assistance with U-Boot Compilation Error

[Sean Anderson]

On 8/26/24 19:18, Reyders Morales wrote:

Dear Tom,

I hope this message finds you well.

My name is Reyders, and I am currently working on compiling U-Boot for
am335x_evm_defconfig. However, I have encountered a compilation error.
I greatly respect the work being done on this project and would be
very grateful for any guidance you could provide.

Details of the Issue:

Environment: Ubuntu 20.04, arm-linux-gnueabihf-gcc 9.4.0
Target Board/Architecture: am335x_evm_defconfig
U-Boot Version: v2024.10-rc3 or main
Error Message:
-
LD  spl/u-boot-spl
arm-linux-gnueabihf-ld.bfd: u-boot-spl section `__u_boot_list' will
not fit in region `.sram'
arm-linux-gnueabihf-ld.bfd: region `.sram' overflowed by 156 bytes
make[1]: *** [scripts/Makefile.spl:532: spl/u-boot-spl] Error 1
make: *** [Makefile:2095: spl/u-boot-spl] Error 2
-



Well, it's just what is says. Your U-Boot doesn't fit in ram.


Additional Context:

I've been able to successfully compile the v2024.07-rc5 version. I
think that this is the last one that works.


Try increasing CONFIG_SPL_MAX_SIZE and then use linux/scripts/bloat-o-meter
to see where the growth is.

But the main thing is that U-Boot is tested with the latest GCC, and older
versions tend to have worse optimization.

The easiest way to get a newer GCC is to download one from [1]. Since you
are on focal, you could also try [2] (but that only provides GCC 11).

[1] https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/
[2] https://launchpad.net/~dristan-viburnums/+archive/ubuntu/toolchain


I was compiling the u-boot just for learning purposes. Sandly, I think
that I do not have the knowledge to fix it. If you can guide me in a
certain direction, I would greatly appreciate it.

In addition, I would greatly appreciate any insights or
recommendations you might have to help resolve this. If you need
further details or clarification on any point, I would be more than
happy to provide them.

Thank you very much for your time and for all the hard work that goes
into maintaining this project. I look forward to any advice you might
offer

Thanks in advance,
Reyders
PD: I wrote to you because I use the get_maintainer.pl script and they
show these names.


Please CC the U-Boot mailing list.

--Sean

Re: [PATCH 09/21] test: Update NAND test to avoid extra macros

[Sean Anderson]

On 8/10/24 16:51, Simon Glass wrote:

Use a single test function with a for() loop instead of using macros to
create multiple test functions. Add a message so it is clear what piece
the test is up to, if it fails.

Signed-off-by: Simon Glass 


Well, the whole reason I wrote it this was was to make sure both tests run
even if one fails. They flashes have different sizes/geometries/ecc and it's
definitely possible for some regression to happen for one but not the other.
Having two test cases makes it easy to see this.

--Sean

Re: [PATCH] spl: fix error handling of spl_fit_read

[Sean Anderson]

On 8/16/24 03:33, mailingli...@johanneskirchmair.de wrote:

From: Johannes Kirchmair 

Returning negative values from spl_fit_read leads to u-boot crashing.
The return value of spl_fit_read is compared with an unsigned value.
Returning negative values leads to the check not detecting the error.
Not detecting the error leads to crashing.

Returning zero in case of an reading error is fine. It indicates that
nothing was red.

Signed-off-by: Johannes Kirchmair 
---
  common/spl/spl_fat.c | 2 +-
  include/spl_load.h   | 4 ++--
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
index bd8aab253a..345bc55149 100644
--- a/common/spl/spl_fat.c
+++ b/common/spl/spl_fat.c
@@ -53,7 +53,7 @@ static ulong spl_fit_read(struct spl_load_info *load, ulong 
file_offset,
  
  	ret = fat_read_file(filename, buf, file_offset, size, &actread);

if (ret)
-   return ret;
+   return 0;
  
  	return actread;

  }
diff --git a/include/spl_load.h b/include/spl_load.h
index 1c2b296c0a..b411d9daa1 100644
--- a/include/spl_load.h
+++ b/include/spl_load.h
@@ -17,8 +17,8 @@ static inline int _spl_load(struct spl_image_info *spl_image,
  {
struct legacy_img_hdr *header =
spl_get_load_buffer(-sizeof(*header), sizeof(*header));
-   ulong base_offset, image_offset, overhead;
-   int read, ret;
+   ulong base_offset, image_offset, overhead, read;
+   int ret;
  
  	read = info->read(info, offset, ALIGN(sizeof(*header),

  spl_get_bl_len(info)), header);


Does [1] fix your problem?

--Sean

[1] https://lore.kernel.org/u-boot/20240731130913.487121-1-fvent...@comcast.net/

[PATCH v3] sandbox: Fix pinmux warnings with non-test devicetrees

[Sean Anderson]

The sandbox pinmux driver is used in the non-test devicetree as well as
the test one. I didn't realize this when I modified the driver for
tests, and so broke the regular use case (which only resulted in
warnings). First, making the pinmux and the UART group available
pre-relocation to avoid ENODEV errors. Then, convert the pin groups and
functions to the new style, adding onewire group as well.

Fixes: 7f0f1806e3a ("test: pinmux: Add test for pin muxing")
Closes: https://source.denx.de/u-boot/u-boot/-/issues/2
Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

Changes in v3:
- Fix "something strange" in the commit message

Changes in v2:
- Only add the pinctrl node pre-relocation, as SPL/VPL try to run dtoc on all
  nodes included in their device trees.

 arch/sandbox/dts/sandbox.dtsi| 14 --
 drivers/pinctrl/pinctrl-sandbox.c|  4 +++-
 include/dt-bindings/pinctrl/sandbox-pinmux.h |  1 +
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index c93ce712894..8a115c503dc 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -231,23 +231,25 @@
};
 
pinctrl {
+   bootph-some-ram;
compatible = "sandbox,pinctrl";
status = "okay";
 
pinctrl_i2c0: i2c0 {
-   groups = "i2c";
-   function = "i2c";
+   groups = "I2C_UART";
+   function = "I2C";
bias-pull-up;
};
 
pinctrl_serial0: uart0 {
-   groups = "serial_a";
-   function = "serial";
+   bootph-some-ram;
+   groups = "I2C_UART";
+   function = "UART";
};
 
pinctrl_onewire0: onewire0 {
-   groups = "w1";
-   function = "w1";
+   pins = "P8";
+   function = "ONEWIRE";
bias-pull-up;
};
};
diff --git a/drivers/pinctrl/pinctrl-sandbox.c 
b/drivers/pinctrl/pinctrl-sandbox.c
index a5d056643a0..f6921b56ceb 100644
--- a/drivers/pinctrl/pinctrl-sandbox.c
+++ b/drivers/pinctrl/pinctrl-sandbox.c
@@ -42,7 +42,7 @@ static const char * const sandbox_pins_muxing[][2] = {
{ "GPIO0", "SPI CS0" },
{ "GPIO1", "SPI CS1" },
{ "GPIO2", "PWM0" },
-   { "GPIO3", "PWM1" },
+   { "GPIO3", "ONEWIRE" },
 };
 
 #define SANDBOX_GROUP_I2C_UART 0
@@ -63,6 +63,7 @@ static const char * const sandbox_functions[] = {
FUNC(GPIO),
FUNC(CS),
FUNC(PWM),
+   FUNC(ONEWIRE),
 #undef FUNC
 };
 
@@ -166,6 +167,7 @@ static int sandbox_pinmux_set(struct udevice *dev, unsigned 
pin_selector,
break;
case SANDBOX_PINMUX_CS:
case SANDBOX_PINMUX_PWM:
+   case SANDBOX_PINMUX_ONEWIRE:
mux = BIT(pin_selector);
break;
default:
diff --git a/include/dt-bindings/pinctrl/sandbox-pinmux.h 
b/include/dt-bindings/pinctrl/sandbox-pinmux.h
index 891af072e52..21c5a1762ab 100644
--- a/include/dt-bindings/pinctrl/sandbox-pinmux.h
+++ b/include/dt-bindings/pinctrl/sandbox-pinmux.h
@@ -13,6 +13,7 @@
 #define SANDBOX_PINMUX_GPIO 4
 #define SANDBOX_PINMUX_CS   5
 #define SANDBOX_PINMUX_PWM  6
+#define SANDBOX_PINMUX_ONEWIRE 7
 
 #define SANDBOX_PINMUX(pin, func) ((func) << 16 | (pin))
 
-- 
2.37.1

[PATCH v2] sandbox: Fix pinmux warnings with non-test devicetrees

[Sean Anderson]

The sandbox pinmux driver is used in the non-test devicetree as well as
the test one. I didn't realize this when I modified the driver for
tests, and so broke the regular use case (which only resulted in
warnings). First, making the pinmux and the UART group available in all
boot phases to avoid ENODEV errors. Then, convert the pin groups and
functions to the new style, adding onewire group as well.

Fixes: 7f0f1806e3a ("test: pinmux: Add test for pin muxing")
Closes: https://source.denx.de/u-boot/u-boot/-/issues/2 Please enter the commit 
message for your changes. Lines starting
Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

Changes in v2:
- Only add the pinctrl node pre-relocation, as SPL/VPL try to run dtoc on all
  nodes included in their device trees.

 arch/sandbox/dts/sandbox.dtsi| 14 --
 drivers/pinctrl/pinctrl-sandbox.c|  4 +++-
 include/dt-bindings/pinctrl/sandbox-pinmux.h |  1 +
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index c93ce712894..8a115c503dc 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -231,23 +231,25 @@
};
 
pinctrl {
+   bootph-some-ram;
compatible = "sandbox,pinctrl";
status = "okay";
 
pinctrl_i2c0: i2c0 {
-   groups = "i2c";
-   function = "i2c";
+   groups = "I2C_UART";
+   function = "I2C";
bias-pull-up;
};
 
pinctrl_serial0: uart0 {
-   groups = "serial_a";
-   function = "serial";
+   bootph-some-ram;
+   groups = "I2C_UART";
+   function = "UART";
};
 
pinctrl_onewire0: onewire0 {
-   groups = "w1";
-   function = "w1";
+   pins = "P8";
+   function = "ONEWIRE";
bias-pull-up;
};
};
diff --git a/drivers/pinctrl/pinctrl-sandbox.c 
b/drivers/pinctrl/pinctrl-sandbox.c
index a5d056643a0..f6921b56ceb 100644
--- a/drivers/pinctrl/pinctrl-sandbox.c
+++ b/drivers/pinctrl/pinctrl-sandbox.c
@@ -42,7 +42,7 @@ static const char * const sandbox_pins_muxing[][2] = {
{ "GPIO0", "SPI CS0" },
{ "GPIO1", "SPI CS1" },
{ "GPIO2", "PWM0" },
-   { "GPIO3", "PWM1" },
+   { "GPIO3", "ONEWIRE" },
 };
 
 #define SANDBOX_GROUP_I2C_UART 0
@@ -63,6 +63,7 @@ static const char * const sandbox_functions[] = {
FUNC(GPIO),
FUNC(CS),
FUNC(PWM),
+   FUNC(ONEWIRE),
 #undef FUNC
 };
 
@@ -166,6 +167,7 @@ static int sandbox_pinmux_set(struct udevice *dev, unsigned 
pin_selector,
break;
case SANDBOX_PINMUX_CS:
case SANDBOX_PINMUX_PWM:
+   case SANDBOX_PINMUX_ONEWIRE:
mux = BIT(pin_selector);
break;
default:
diff --git a/include/dt-bindings/pinctrl/sandbox-pinmux.h 
b/include/dt-bindings/pinctrl/sandbox-pinmux.h
index 891af072e52..21c5a1762ab 100644
--- a/include/dt-bindings/pinctrl/sandbox-pinmux.h
+++ b/include/dt-bindings/pinctrl/sandbox-pinmux.h
@@ -13,6 +13,7 @@
 #define SANDBOX_PINMUX_GPIO 4
 #define SANDBOX_PINMUX_CS   5
 #define SANDBOX_PINMUX_PWM  6
+#define SANDBOX_PINMUX_ONEWIRE 7
 
 #define SANDBOX_PINMUX(pin, func) ((func) << 16 | (pin))
 
-- 
2.37.1

[PATCH 2/2] arm: zynqmp: Enable non-invasive CCI-400 PMU debug

[Sean Anderson]

Set NIDEN, enabling non-invasive debug for the CCI-400 PMU. Otherwise,
the PMU is effectively disabled.

Signed-off-by: Sean Anderson 
---

 arch/arm/mach-zynqmp/include/mach/hardware.h | 3 +++
 board/xilinx/zynqmp/zynqmp.c | 4 
 2 files changed, 7 insertions(+)

diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
b/arch/arm/mach-zynqmp/include/mach/hardware.h
index f1514d6a869..51eab3509b8 100644
--- a/arch/arm/mach-zynqmp/include/mach/hardware.h
+++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
@@ -128,6 +128,9 @@ struct crfapb_regs {
 
 #define crfapb_base ((struct crfapb_regs *)ZYNQMP_CRF_APB_BASEADDR)
 
+#define ZYNQMP_CCI_REG_CCI_MISC_CTRL   0xFD5E0040
+#define ZYNQMP_CCI_REG_CCI_MISC_CTRL_NIDEN BIT(1)
+
 #define ZYNQMP_APU_BASEADDR0xFD5C
 
 struct apu_regs {
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 94a66684af2..e0ff2607952 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -76,6 +76,10 @@ int __maybe_unused psu_uboot_init(void)
writel(0x04920492, ZYNQMP_IOU_SECURE_SLCR);
writel(0x04920492, ZYNQMP_IOU_SECURE_SLCR + 4);
 
+   /* Enable CCI PMU events */
+   writel(ZYNQMP_CCI_REG_CCI_MISC_CTRL_NIDEN,
+  ZYNQMP_CCI_REG_CCI_MISC_CTRL);
+
/* Delay is required for clocks to be propagated */
udelay(100);

-- 
2.35.1.1320.gc452695387.dirty

[PATCH 1/2] zynqmp: Disable secure access for boot devices

[Sean Anderson]

Boot devices (QSPI, MMC, NAND, and Ethernet) use secure access for DMA
by default. As this causes problems when using the SMMU [1], configure
them for normal access instead.

[1] https://support.xilinx.com/s/article/72164

Signed-off-by: Sean Anderson 
---

 arch/arm/mach-zynqmp/include/mach/hardware.h | 2 ++
 board/xilinx/zynqmp/zynqmp.c | 4 
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
b/arch/arm/mach-zynqmp/include/mach/hardware.h
index 8cb6494e52c..f1514d6a869 100644
--- a/arch/arm/mach-zynqmp/include/mach/hardware.h
+++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
@@ -63,6 +63,8 @@ struct crlapb_regs {
 
 #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
 
+#define ZYNQMP_IOU_SECURE_SLCR 0xFF24
+
 #define ZYNQMP_IOU_SCNTR_SECURE0xFF26
 #define ZYNQMP_IOU_SCNTR_COUNTER_CONTROL_REGISTER_EN   0x1
 #define ZYNQMP_IOU_SCNTR_COUNTER_CONTROL_REGISTER_HDBG 0x2
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index b4c15b041cc..94a66684af2 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -72,6 +72,10 @@ int __maybe_unused psu_uboot_init(void)
writel(ZYNQMP_PS_SYSMON_ANALOG_BUS_VAL,
   ZYNQMP_AMS_PS_SYSMON_ANALOG_BUS);
 
+   /* Disable secure access for boot devices */
+   writel(0x04920492, ZYNQMP_IOU_SECURE_SLCR);
+   writel(0x04920492, ZYNQMP_IOU_SECURE_SLCR + 4);
+
/* Delay is required for clocks to be propagated */
udelay(100);

-- 
2.35.1.1320.gc452695387.dirty

[PATCH 0/2] arm: zynqmp: Initialize some registers at boot

[Sean Anderson]

These patches are independent in intent, but they modify adjacent
lines so I have sent them as a series.


Sean Anderson (2):
  zynqmp: Disable secure access for boot devices
  arm: zynqmp: Enable non-invasive CCI-400 PMU debug

 arch/arm/mach-zynqmp/include/mach/hardware.h | 5 +
 board/xilinx/zynqmp/zynqmp.c | 8 
 2 files changed, 13 insertions(+)

-- 
2.35.1.1320.gc452695387.dirty

[PATCH] pinmux: generic: Use ENOENT instead of ENOSYS

[Sean Anderson]

ENOSYS should only be used when a subsystem is completely absent.
Convert its use in pinctrl-generic to ENOENT, which better reflects the
error condition (that a function/group/pin is missing).

Signed-off-by: Sean Anderson 
---

 drivers/pinctrl/pinctrl-generic.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-generic.c 
b/drivers/pinctrl/pinctrl-generic.c
index 2464acf0b85..81a9327eb35 100644
--- a/drivers/pinctrl/pinctrl-generic.c
+++ b/drivers/pinctrl/pinctrl-generic.c
@@ -22,7 +22,7 @@ static int pinctrl_pin_name_to_selector(struct udevice *dev, 
const char *pin)
 
if (!ops->get_pins_count || !ops->get_pin_name) {
dev_dbg(dev, "get_pins_count or get_pin_name missing\n");
-   return -ENOSYS;
+   return -ENOENT;
}
 
npins = ops->get_pins_count(dev);
@@ -35,7 +35,7 @@ static int pinctrl_pin_name_to_selector(struct udevice *dev, 
const char *pin)
return selector;
}
 
-   return -ENOSYS;
+   return -ENOENT;
 }
 
 /**
@@ -53,7 +53,7 @@ static int pinctrl_group_name_to_selector(struct udevice *dev,
 
if (!ops->get_groups_count || !ops->get_group_name) {
dev_dbg(dev, "get_groups_count or get_group_name missing\n");
-   return -ENOSYS;
+   return -ENOENT;
}
 
ngroups = ops->get_groups_count(dev);
@@ -66,7 +66,7 @@ static int pinctrl_group_name_to_selector(struct udevice *dev,
return selector;
}
 
-   return -ENOSYS;
+   return -ENOENT;
 }
 
 #if CONFIG_IS_ENABLED(PINMUX)
@@ -86,7 +86,7 @@ static int pinmux_func_name_to_selector(struct udevice *dev,
if (!ops->get_functions_count || !ops->get_function_name) {
dev_dbg(dev,
"get_functions_count or get_function_name missing\n");
-   return -ENOSYS;
+   return -ENOENT;
}
 
nfuncs = ops->get_functions_count(dev);
@@ -99,7 +99,7 @@ static int pinmux_func_name_to_selector(struct udevice *dev,
return selector;
}
 
-   return -ENOSYS;
+   return -ENOENT;
 }
 
 /**
@@ -119,14 +119,14 @@ static int pinmux_enable_setting(struct udevice *dev, 
bool is_group,
if (is_group) {
if (!ops->pinmux_group_set) {
dev_dbg(dev, "pinmux_group_set op missing\n");
-   return -ENOSYS;
+   return -ENOENT;
}
 
return ops->pinmux_group_set(dev, selector, func_selector);
} else {
if (!ops->pinmux_set) {
dev_dbg(dev, "pinmux_set op missing\n");
-   return -ENOSYS;
+   return -ENOENT;
}
return ops->pinmux_set(dev, selector, func_selector);
}
@@ -162,7 +162,7 @@ static int pinconf_prop_name_to_param(struct udevice *dev,
 
if (!ops->pinconf_num_params || !ops->pinconf_params) {
dev_dbg(dev, "pinconf_num_params or pinconf_params missing\n");
-   return -ENOSYS;
+   return -ENOENT;
}
 
p = ops->pinconf_params;
@@ -176,7 +176,7 @@ static int pinconf_prop_name_to_param(struct udevice *dev,
}
}
 
-   return -ENOSYS;
+   return -ENOENT;
 }
 
 /**
@@ -198,7 +198,7 @@ static int pinconf_enable_setting(struct udevice *dev, bool 
is_group,
if (is_group) {
if (!ops->pinconf_group_set) {
dev_dbg(dev, "pinconf_group_set op missing\n");
-   return -ENOSYS;
+   return -ENOENT;
}
 
return ops->pinconf_group_set(dev, selector, param,
@@ -206,7 +206,7 @@ static int pinconf_enable_setting(struct udevice *dev, bool 
is_group,
} else {
if (!ops->pinconf_set) {
dev_dbg(dev, "pinconf_set op missing\n");
-   return -ENOSYS;
+   return -ENOENT;
}
return ops->pinconf_set(dev, selector, param, argument);
}
@@ -215,7 +215,7 @@ static int pinconf_enable_setting(struct udevice *dev, bool 
is_group,
 static int pinconf_prop_name_to_param(struct udevice *dev,
  const char *property, u32 *default_value)
 {
-   return -ENOSYS;
+   return -ENOENT;
 }
 
 static int pinconf_enable_setting(struct udevice *dev, bool is_group,
-- 
2.37.1

[PATCH] sandbox: Fix pinmux warnings with non-test devicetrees

[Sean Anderson]

The sandbox pinmux driver is used in the non-test devicetree as well as
the test one. I didn't realize this when I modified the driver for
tests, and so broke the regular use case (which only resulted in
warnings). First, making the pinmux and the UART group available in all
boot phases to avoid ENODEV errors. Then, convert the pin groups and
functions to the new style, adding onewire group as well.

Fixes: 7f0f1806e3a ("test: pinmux: Add test for pin muxing")
Signed-off-by: Sean Anderson 
---

 arch/sandbox/dts/sandbox.dtsi| 14 --
 drivers/pinctrl/pinctrl-sandbox.c|  4 +++-
 include/dt-bindings/pinctrl/sandbox-pinmux.h |  1 +
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index c93ce712894..06ba4f031fa 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -231,23 +231,25 @@
};
 
pinctrl {
+   bootph-all;
compatible = "sandbox,pinctrl";
status = "okay";
 
pinctrl_i2c0: i2c0 {
-   groups = "i2c";
-   function = "i2c";
+   groups = "I2C_UART";
+   function = "I2C";
bias-pull-up;
};
 
pinctrl_serial0: uart0 {
-   groups = "serial_a";
-   function = "serial";
+   bootph-all;
+   groups = "I2C_UART";
+   function = "UART";
};
 
pinctrl_onewire0: onewire0 {
-   groups = "w1";
-   function = "w1";
+   pins = "P8";
+   function = "ONEWIRE";
bias-pull-up;
};
};
diff --git a/drivers/pinctrl/pinctrl-sandbox.c 
b/drivers/pinctrl/pinctrl-sandbox.c
index a5d056643a0..f6921b56ceb 100644
--- a/drivers/pinctrl/pinctrl-sandbox.c
+++ b/drivers/pinctrl/pinctrl-sandbox.c
@@ -42,7 +42,7 @@ static const char * const sandbox_pins_muxing[][2] = {
{ "GPIO0", "SPI CS0" },
{ "GPIO1", "SPI CS1" },
{ "GPIO2", "PWM0" },
-   { "GPIO3", "PWM1" },
+   { "GPIO3", "ONEWIRE" },
 };
 
 #define SANDBOX_GROUP_I2C_UART 0
@@ -63,6 +63,7 @@ static const char * const sandbox_functions[] = {
FUNC(GPIO),
FUNC(CS),
FUNC(PWM),
+   FUNC(ONEWIRE),
 #undef FUNC
 };
 
@@ -166,6 +167,7 @@ static int sandbox_pinmux_set(struct udevice *dev, unsigned 
pin_selector,
break;
case SANDBOX_PINMUX_CS:
case SANDBOX_PINMUX_PWM:
+   case SANDBOX_PINMUX_ONEWIRE:
mux = BIT(pin_selector);
break;
default:
diff --git a/include/dt-bindings/pinctrl/sandbox-pinmux.h 
b/include/dt-bindings/pinctrl/sandbox-pinmux.h
index 891af072e52..21c5a1762ab 100644
--- a/include/dt-bindings/pinctrl/sandbox-pinmux.h
+++ b/include/dt-bindings/pinctrl/sandbox-pinmux.h
@@ -13,6 +13,7 @@
 #define SANDBOX_PINMUX_GPIO 4
 #define SANDBOX_PINMUX_CS   5
 #define SANDBOX_PINMUX_PWM  6
+#define SANDBOX_PINMUX_ONEWIRE 7
 
 #define SANDBOX_PINMUX(pin, func) ((func) << 16 | (pin))
 
-- 
2.37.1

Re: pinctrl warning on sandbox

[Sean Anderson]

Hi Simon,

On 8/7/24 19:17, Simon Glass wrote:

Hi Sean,

I was looking at [1] and bisected the message to this commit:

7f0f1806e3a (refs/bisect/bad) test: pinmux: Add test for pin muxing

I'm not quite sure what is going on, but could you please take a look?

Regards,
Simon

[1] https://source.denx.de/u-boot/u-boot/-/issues/2


That test run is pretty old. Does the error always occur during EFI tests?
You said in IRC it occurs on current master; can you send a link?

--Sean

Re: [PATCH v2] mmc: fix signed vs unsigned compare in read check in _spl_load()

[Sean Anderson]

On 7/31/24 09:09, Franco Venturi wrote:

Fix signed vs unsigned compare in read check in _spl_load()

Issue: when info->read() returns a negative value because of an error,
the comparison of 'read' (signed) with 'sizeof(*header)'
(unsigned silently converts the negative value into a very
large unsigned value and the check on the error condition
always return false, i.e. the error is not detected
Symptoms: if spl_load_image_fat() is unable to find the file 'uImage',
   the SPL phase of the boot process just hangs after displaying
   the following line:
   Trying to boot from MMC1
Fix: cast 'sizeof(*header)' to int so the compare is now between
  signed types
Reference: 
https://stackoverflow.com/questions/17293749/sizeof-operator-in-if-statement

Signed-off-by: Franco Venturi 
---

(no changes since v1)

  include/spl_load.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/spl_load.h b/include/spl_load.h
index 1c2b296c0a..83db381202 100644
--- a/include/spl_load.h
+++ b/include/spl_load.h
@@ -22,7 +22,7 @@ static inline int _spl_load(struct spl_image_info *spl_image,
  
  	read = info->read(info, offset, ALIGN(sizeof(*header),

  spl_get_bl_len(info)), header);
-   if (read < sizeof(*header))
+   if (read < (int)sizeof(*header))
return -EIO;
  
  	if (image_get_magic(header) == FDT_MAGIC) {


Reviewed-by: Sean Anderson 

Re: [PATCH v1] mmc: fix signed vs unsigned compare in read check in _spl_load()

[Sean Anderson]

Hi Franco,

On 7/30/24 09:30, Franco Venturi wrote:

Fix signed vs unsigned compare in read check in _spl_load()

Issue: when info->read() returns a negative value because of an error,
the comparison of 'read' (signed) with 'sizeof(*header)'
(unsigned silently converts the negative value into a very
large unsigned value and the check on the error condition
always return false, i.e. the error is not detected
Symptoms: if spl_load_image_fat() is unable to find the file 'uImage',
   the SPL phase of the boot process just hangs after displaying
   the following line:
   Trying to boot from MMC1
Fix: first check if 'read' is negative then check its value against
  'sizeof(*header)'
Reference: 
https://stackoverflow.com/questions/17293749/sizeof-operator-in-if-statement

Signed-off-by: Franco Venturi 
---

  include/spl_load.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/spl_load.h b/include/spl_load.h
index 1c2b296c0a..1e05599d29 100644
--- a/include/spl_load.h
+++ b/include/spl_load.h
@@ -22,7 +22,7 @@ static inline int _spl_load(struct spl_image_info *spl_image,
  
  	read = info->read(info, offset, ALIGN(sizeof(*header),

  spl_get_bl_len(info)), header);
-   if (read < sizeof(*header))
+   if (read < 0 || read < sizeof(*header))
return -EIO;
  
  	if (image_get_magic(header) == FDT_MAGIC) {


Since read and int and sizeof(*header) < INT_MAX, I think the best solution is
to cast the latter to an int.

--Sean

Re: [PATCH 11/14] spl: mmc: Try to clean up raw-mode options

[Sean Anderson]

On 7/20/24 02:17, Simon Glass wrote:

Make the raw-mode options depend on SPL_SYS_MMCSD_RAW_MODE in a more
direct way. This makes it easier to understand the options with
'make menuconfig'.

There are three different ways of specifying the offset:

- sector offset
- partition number
- partition type

So make these a choice, so it is more obvious what is going on.

Update existing boards to enable SPL_SYS_MMCSD_RAW_MODE where needed.

Signed-off-by: Simon Glass 
---

  arch/arm/mach-imx/imx8m/soc.c |  2 +
  arch/arm/mach-imx/spl_imx_romapi.c| 13 -
  .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c   |  4 +-
  common/spl/Kconfig| 52 +++
  configs/am335x_guardian_defconfig |  2 +-
  configs/am335x_pdu001_defconfig   |  2 +-
  configs/am3517_evm_defconfig  |  2 +-
  configs/am62ax_evm_a53_defconfig  |  1 +
  configs/am62ax_evm_r5_defconfig   |  1 +
  configs/am62px_evm_a53_defconfig  |  1 +
  configs/am62px_evm_r5_defconfig   |  1 +
  configs/am62x_beagleplay_a53_defconfig|  1 +
  configs/am62x_beagleplay_r5_defconfig |  1 +
  configs/am62x_evm_a53_defconfig   |  1 +
  configs/am62x_evm_r5_defconfig|  1 +
  configs/am64x_evm_a53_defconfig   |  1 +
  configs/am64x_evm_r5_defconfig|  1 +
  configs/am65x_evm_a53_defconfig   |  1 +
  configs/am65x_evm_r5_defconfig|  1 +
  configs/brppt2_defconfig  |  2 +-
  configs/brsmarc1_defconfig|  2 +-
  configs/cgtqmx8_defconfig |  1 +
  configs/chromebit_mickey_defconfig|  2 +-
  configs/chromebook_jerry_defconfig|  2 +-
  configs/chromebook_minnie_defconfig   |  2 +-
  configs/chromebook_speedy_defconfig   |  2 +-
  configs/ci20_mmc_defconfig|  1 +
  configs/da850evm_defconfig|  2 +-
  configs/da850evm_nand_defconfig   |  2 +-
  configs/deneb_defconfig   |  1 +
  configs/display5_defconfig|  2 +-
  configs/display5_factory_defconfig|  2 +-
  configs/draco-rastaban_defconfig  |  2 +-
  configs/draco-thuban_defconfig|  2 +-
  .../gardena-smart-gateway-at91sam_defconfig   |  2 +-
  configs/giedi_defconfig   |  1 +
  configs/imx28_xea_defconfig   |  1 +
  configs/imx28_xea_sb_defconfig|  1 +
  configs/imx6q_logic_defconfig |  2 +-
  configs/imx8mm-cl-iot-gate-optee_defconfig|  1 +
  configs/imx8mm-cl-iot-gate_defconfig  |  1 +
  configs/imx8mm-icore-mx8mm-ctouch2_defconfig  |  1 +
  configs/imx8mm-icore-mx8mm-edimm2.2_defconfig |  1 +
  configs/imx8mm-mx8menlo_defconfig |  1 +
  configs/imx8mm-phygate-tauri-l_defconfig  |  1 +
  configs/imx8mm_beacon_defconfig   |  1 +
  configs/imx8mm_beacon_fspi_defconfig  |  1 +
  configs/imx8mm_data_modul_edm_sbc_defconfig   |  1 +
  configs/imx8mm_evk_defconfig  |  1 +
  configs/imx8mm_evk_fspi_defconfig |  1 +
  configs/imx8mm_phg_defconfig  |  1 +
  configs/imx8mm_venice_defconfig   |  1 +
  configs/imx8mn_beacon_2g_defconfig|  1 +
  configs/imx8mn_beacon_defconfig   |  1 +
  configs/imx8mn_beacon_fspi_defconfig  |  1 +
  configs/imx8mn_bsh_smm_s2_defconfig   |  1 +
  configs/imx8mn_bsh_smm_s2pro_defconfig|  1 +
  configs/imx8mn_ddr4_evk_defconfig |  1 +
  configs/imx8mn_evk_defconfig  |  1 +
  configs/imx8mn_var_som_defconfig  |  1 +
  configs/imx8mn_venice_defconfig   |  1 +
  configs/imx8mp-icore-mx8mp-edimm2.2_defconfig |  1 +
  configs/imx8mp_beacon_defconfig   |  1 +
  configs/imx8mp_data_modul_edm_sbc_defconfig   |  1 +
  configs/imx8mp_debix_model_a_defconfig|  1 +
  configs/imx8mp_dhcom_pdk2_defconfig   |  1 +
  configs/imx8mp_dhcom_pdk3_defconfig   |  1 +
  configs/imx8mp_evk_defconfig  |  1 +
  configs/imx8mp_rsb3720a1_4G_defconfig |  2 +
  configs/imx8mp_rsb3720a1_6G_defconfig |  1 +
  configs/imx8mp_venice_defconfig   |  1 +
  configs/imx8mq_cm_defconfig   |  1 +
  configs/imx8mq_evk_defconfig  |  1 +
  configs/imx8mq_phanbell_defconfig |  1 +
  configs/imx8mq_reform2_defconfig  |  1 +
  configs/imx8qm_mek_defconfig  |  1 +
  configs/imx8qxp_mek_defconfig |  1 +
  configs/imx8ulp_evk_defconfig |  1 +
  configs/imx93-phyboard-segin_defconfig|  1 +
  configs/imx93_11x11_evk_defconfig |  1 +
  configs/imx93_11x11_evk_ld_defconfig  |  1 +
  configs/imx93_var_som_defconfi

Re: [PATCH 14/14] blk: Correct comment for blk_get_devnum_by_uclass_idname()

[Sean Anderson]

On 7/20/24 02:17, Simon Glass wrote:

Update the comment to match the function. Fix the indentation while we
are here.

Signed-off-by: Simon Glass 
---

  include/blk.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/blk.h b/include/blk.h
index 7c7cf7f2b10..1fc9a5b8471 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -650,7 +650,7 @@ struct blk_driver *blk_driver_lookup_type(int uclass_id);
  struct blk_desc *blk_get_devnum_by_uclass_id(enum uclass_id uclass_id, int 
devnum);
  
  /**

- * blk_get_devnum_by_uclass_id() - Get a block device by type name, and number
+ * blk_get_devnum_by_uclass_idname() - Get block device by type name and number
   *
   * This looks up the block device type based on @uclass_idname, then calls
   * blk_get_devnum_by_uclass_id().
@@ -660,7 +660,7 @@ struct blk_desc *blk_get_devnum_by_uclass_id(enum uclass_id 
uclass_id, int devnu
   * Return: point to block device descriptor, or NULL if not found
   */
  struct blk_desc *blk_get_devnum_by_uclass_idname(const char *uclass_idname,
-   int devnum);
+int devnum);
  
  /**

   * blk_dselect_hwpart() - select a hardware partition


Reviewed-by: Sean Anderson 

Re: [PATCH 13/14] spl: Create a function to init spl_load_info

[Sean Anderson]

On 7/20/24 02:17, Simon Glass wrote:

Rather than having every caller set this up individually, create a
common init function. This allows new fields to be added without the
risk of them being left uninited.



What is the code size impact of this?

IIRC the reason why I didn't do this is to avoid growing the code in
cases where priv is unused.

Although in some places there are memsets which should be removed anyway.


Signed-off-by: Simon Glass 
---

  arch/arm/mach-imx/spl_imx_romapi.c  | 14 ++---
  arch/arm/mach-sunxi/spl_spi_sunxi.c |  3 +-
  common/spl/spl_blk_fs.c |  9 ++
  common/spl/spl_ext.c|  3 +-
  common/spl/spl_fat.c| 10 +++
  common/spl/spl_mmc.c|  4 +--
  common/spl/spl_nand.c   |  4 +--
  common/spl/spl_net.c|  3 +-
  common/spl/spl_nor.c|  6 ++--
  common/spl/spl_ram.c|  3 +-
  common/spl/spl_semihosting.c|  4 +--
  common/spl/spl_spi.c|  4 +--
  common/spl/spl_ymodem.c |  4 +--
  drivers/usb/gadget/f_sdp.c  |  8 ++
  include/spl.h   | 44 -
  test/image/spl_load.c   |  4 +--
  test/image/spl_load_os.c|  6 +---
  17 files changed, 55 insertions(+), 78 deletions(-)

diff --git a/arch/arm/mach-imx/spl_imx_romapi.c 
b/arch/arm/mach-imx/spl_imx_romapi.c
index bdaea439d7f..3982f4cca18 100644
--- a/arch/arm/mach-imx/spl_imx_romapi.c
+++ b/arch/arm/mach-imx/spl_imx_romapi.c
@@ -108,18 +108,13 @@ static int spl_romapi_load_image_seekable(struct 
spl_image_info *spl_image,
if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) && image_get_magic(header) == 
FDT_MAGIC) {
struct spl_load_info load;
  
-		memset(&load, 0, sizeof(load));

-   spl_set_bl_len(&load, pagesize);
-   load.read = spl_romapi_read_seekable;
+   spl_load_init(&load, spl_romapi_read_seekable, NULL, pagesize);
return spl_load_simple_fit(spl_image, &load, offset, header);
} else if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER) &&
   valid_container_hdr((void *)header)) {
struct spl_load_info load;
  
-		memset(&load, 0, sizeof(load));

-   spl_set_bl_len(&load, pagesize);
-   load.read = spl_romapi_read_seekable;
-
+   spl_load_init(&load, spl_romapi_read_seekable, NULL, pagesize);
ret = spl_load_imx_container(spl_image, &load, offset);
} else {
/* TODO */
@@ -341,10 +336,7 @@ static int spl_romapi_load_image_stream(struct 
spl_image_info *spl_image,
ss.end = p;
ss.pagesize = pagesize;
  
-		memset(&load, 0, sizeof(load));

-   spl_set_bl_len(&load, 1);
-   load.read = spl_romapi_read_stream;
-   load.priv = &ss;
+   spl_load_init(&load, spl_romapi_read_stream, &ss, 1);
  
  		return spl_load_simple_fit(spl_image, &load, (ulong)phdr, phdr);

}
diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c 
b/arch/arm/mach-sunxi/spl_spi_sunxi.c
index d7abdc2e401..5f72e809952 100644
--- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
+++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
@@ -390,8 +390,7 @@ static int spl_spi_load_image(struct spl_image_info 
*spl_image,
struct spl_load_info load;
  
  		debug("Found FIT image\n");

-   spl_set_bl_len(&load, 1);
-   load.read = spi_load_read;
+   spl_load_init(&load, spi_load_read, NULL, 1);
ret = spl_load_simple_fit(spl_image, &load,
  load_offset, header);
} else {
diff --git a/common/spl/spl_blk_fs.c b/common/spl/spl_blk_fs.c
index bc551c5c074..bbf90a9741e 100644
--- a/common/spl/spl_blk_fs.c
+++ b/common/spl/spl_blk_fs.c
@@ -80,11 +80,8 @@ int spl_blk_load_image(struct spl_image_info *spl_image,
return ret;
}
  
-	load.read = spl_fit_read;

-   if (IS_ENABLED(CONFIG_SPL_FS_FAT_DMA_ALIGN))
-   spl_set_bl_len(&load, ARCH_DMA_MINALIGN);
-   else
-   spl_set_bl_len(&load, 1);
-   load.priv = &dev;
+   spl_load_init(&load, spl_fit_read, &dev,
+ IS_ENABLED(CONFIG_SPL_FS_FAT_DMA_ALIGN) ?
+ ARCH_DMA_MINALIGN : 1);
return spl_load(spl_image, bootdev, &load, filesize, 0);
  }
diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
index 76f49a5a8a6..c5478820a9b 100644
--- a/common/spl/spl_ext.c
+++ b/common/spl/spl_ext.c
@@ -51,8 +51,7 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
goto end;
}
  
-	spl_set_bl_len(&load, 1);

-   load.read = spl_fit_read;
+   spl_load_init(&load, spl_fit_read, NULL, 1);
err = spl_load(spl_image, bootdev, &load, filelen, 0);
  
  end:

diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
index bd8aab253a9..fce45

Re: [PATCH 12/14] spl: Use unified inline functions for spl_load_info

[Sean Anderson]

On 7/20/24 02:17, Simon Glass wrote:

Rather than declaring completely separate functions, put the code for
each case into the same function. This makes it easier to read.

Signed-off-by: Simon Glass 
---

  include/spl.h | 18 +++---
  1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/include/spl.h b/include/spl.h
index 5dfdf778d2d..2f6a3e64c10 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -306,31 +306,27 @@ struct spl_load_info {
  void *buf);
  #if IS_ENABLED(CONFIG_SPL_LOAD_BLOCK)
int bl_len;
+#endif
  };
  
  static inline int spl_get_bl_len(struct spl_load_info *info)

  {
+#if IS_ENABLED(CONFIG_SPL_LOAD_BLOCK)
return info->bl_len;
-}
-
-static inline void spl_set_bl_len(struct spl_load_info *info, int bl_len)
-{
-   info->bl_len = bl_len;
-}
  #else
-};
-
-static inline int spl_get_bl_len(struct spl_load_info *info)
-{
return 1;
+#endif
  }
  
  static inline void spl_set_bl_len(struct spl_load_info *info, int bl_len)

  {
+#if IS_ENABLED(CONFIG_SPL_LOAD_BLOCK)
+   info->bl_len = bl_len;
+#else
if (bl_len != 1)
panic("CONFIG_SPL_LOAD_BLOCK not enabled");
-}
  #endif
+}
  
  /*

   * We need to know the position of U-Boot in memory so we can jump to it. We


Reviewed-by: Sean Anderson 

Re: [PATCH 10/14] spl: mmc: Adjust args of spl_mmc_find_device()

[Sean Anderson]

On 7/20/24 02:17, Simon Glass wrote:

At present spl_mmc_load() is the only caller of this function, passing
it a boot_device, an index into the available MMC devices. Pass the
device number instead, since it is known by the caller and simplifies
the code.

Signed-off-by: Simon Glass 
---

  common/spl/spl_mmc.c | 12 
  1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index 70e376c0ed8..3cb4b698f7f 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -58,7 +58,7 @@ int mmc_load_image_raw_sector(struct spl_image_info 
*spl_image,
return 0;
  }
  
-static int spl_mmc_get_device_index(u32 boot_device)

+static int spl_mmc_get_device_index(int boot_device)


This should remain a u32 (or more properly a "uint"...)


  {
switch (boot_device) {
case BOOT_DEVICE_MMC1:
@@ -73,13 +73,9 @@ static int spl_mmc_get_device_index(u32 boot_device)
return -ENODEV;
  }
  
-static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)

+static int spl_mmc_find_device(struct mmc **mmcp, int mmc_dev)
  {
-   int ret, mmc_dev;
-
-   mmc_dev = spl_mmc_get_device_index(boot_device);
-   if (mmc_dev < 0)
-   return mmc_dev;
+   int ret;
  
  #if CONFIG_IS_ENABLED(DM_MMC)

ret = mmc_init_device(mmc_dev);
@@ -349,7 +345,7 @@ int spl_mmc_load(struct spl_image_info *spl_image,
/* Perform peripheral init only once for an mmc device */
mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
if (!mmc || spl_mmc_get_mmc_devnum(mmc) != mmc_dev) {
-   ret = spl_mmc_find_device(&mmc, bootdev->boot_device);
+   ret = spl_mmc_find_device(&mmc, mmc_dev);
if (ret)
return ret;
  


Aside from the above,

Reviewed-by: Sean Anderson 

Re: [PATCH 09/14] spl: mmc: Handle error codes consistently

[Sean Anderson]

On 7/20/24 02:17, Simon Glass wrote:

Use 'ret' as the return code, since it may not be an error and this is
the common name in U-Boot. Make sure to return the error code when
given, rather than transforming it into -1 (-EPERM).


Does this transformation affect code size?


Signed-off-by: Simon Glass 
---

  common/spl/spl_mmc.c | 129 ++-
  1 file changed, 65 insertions(+), 64 deletions(-)

diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index ddc85aaeda7..70e376c0ed8 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -52,7 +52,7 @@ int mmc_load_image_raw_sector(struct spl_image_info 
*spl_image,
ret = spl_load(spl_image, bootdev, &load, 0, sector << bd->log2blksz);
if (ret) {
puts("mmc_load_image_raw_sector: mmc block read error\n");
-   return -1;
+   return ret;
}
  
  	return 0;

@@ -75,27 +75,27 @@ static int spl_mmc_get_device_index(u32 boot_device)
  
  static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)

  {
-   int err, mmc_dev;
+   int ret, mmc_dev;
  
  	mmc_dev = spl_mmc_get_device_index(boot_device);

if (mmc_dev < 0)
return mmc_dev;
  
  #if CONFIG_IS_ENABLED(DM_MMC)

-   err = mmc_init_device(mmc_dev);
+   ret = mmc_init_device(mmc_dev);
  #else
-   err = mmc_initialize(NULL);
+   ret = mmc_initialize(NULL);
  #endif /* DM_MMC */
-   if (err) {
-   printf("spl: could not initialize mmc. error: %d\n", err);
-   return err;
+   if (ret) {
+   printf("spl: could not initialize mmc. error: %d\n", ret);
+   return ret;
}
*mmcp = find_mmc_device(mmc_dev);
-   err = *mmcp ? 0 : -ENODEV;
-   if (err) {
+   ret = *mmcp ? 0 : -ENODEV;
+   if (ret) {
printf("spl: could not find mmc device %d. error: %d\n",
-  mmc_dev, err);
-   return err;
+  mmc_dev, ret);
+   return ret;
}
  
  	return 0;

@@ -108,14 +108,14 @@ static int mmc_load_image_raw_partition(struct 
spl_image_info *spl_image,
unsigned long sector)
  {
struct disk_partition info;
-   int err;
+   int ret;
  
  #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE

int type_part;
/* Only support MBR so DOS_ENTRY_NUMBERS */
for (type_part = 1; type_part <= DOS_ENTRY_NUMBERS; type_part++) {
-   err = part_get_info(mmc_get_blk_desc(mmc), type_part, &info);
-   if (err)
+   ret = part_get_info(mmc_get_blk_desc(mmc), type_part, &info);
+   if (ret)
continue;
if (info.sys_ind ==
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE) {
@@ -125,10 +125,10 @@ static int mmc_load_image_raw_partition(struct 
spl_image_info *spl_image,
}
  #endif
  
-	err = part_get_info(mmc_get_blk_desc(mmc), partition, &info);

-   if (err) {
+   ret = part_get_info(mmc_get_blk_desc(mmc), partition, &info);
+   if (ret) {
puts("spl: partition error\n");
-   return -err;
+   return ret;
}
  
  #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR

@@ -155,7 +155,7 @@ static int mmc_load_image_raw_os(struct spl_image_info 
*spl_image,
(void *)CONFIG_SPL_PAYLOAD_ARGS_ADDR);
if (count != CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS) {
puts("mmc_load_image_raw_os: mmc block read error\n");
-   return -1;
+   return -EIO;
}
  #endif/* CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTOR */
  
@@ -193,7 +193,7 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image,

  struct mmc *mmc,
  const char *filename)
  {
-   int err = -ENOSYS;
+   int ret = -ENOSYS;
  
  	__maybe_unused int partition = CONFIG_SYS_MMCSD_FS_BOOT_PARTITION;
  
@@ -202,8 +202,8 @@ static int spl_mmc_do_fs_boot(struct spl_image_info *spl_image,

struct disk_partition info;
debug("Checking for the first MBR bootable partition\n");
for (int type_part = 1; type_part <= DOS_ENTRY_NUMBERS; 
type_part++) {
-   err = part_get_info(mmc_get_blk_desc(mmc), type_part, 
&info);
-   if (err)
+   ret = part_get_info(mmc_get_blk_desc(mmc), type_part, 
&info);
+   if (ret)
continue;
debug("Partition %d is of type %d and bootable=%d\n", 
type_part, info.sys_ind, info.bootable);
if (info.bootable != 0) {
@@ -221,40 +221,40 @@ static int spl_mmc_do_fs_boot(struct spl_image_info 
*spl_image,
  
  #ifdef CONFIG_SPL_FS_FAT

if (!spl_start_uboot()) {
-   err = spl

Re: [PATCH 08/14] spl: mmc: Drop checks for CONFIG_SPL_LIBCOMMON_SUPPORT

[Sean Anderson]

On 7/20/24 02:17, Simon Glass wrote:

This check is not needed now, since printf() resolved to nothing if not
available. Drop the #ifdefs

Signed-off-by: Simon Glass 
---

  common/spl/spl_mmc.c | 20 +---
  1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index ccab0be4be2..ddc85aaeda7 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -51,9 +51,7 @@ int mmc_load_image_raw_sector(struct spl_image_info 
*spl_image,
load.read = h_spl_load_read;
ret = spl_load(spl_image, bootdev, &load, 0, sector << bd->log2blksz);
if (ret) {
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
puts("mmc_load_image_raw_sector: mmc block read error\n");
-#endif
return -1;
}
  
@@ -70,9 +68,7 @@ static int spl_mmc_get_device_index(u32 boot_device)

return 1;
}
  
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT

printf("spl: unsupported mmc boot device.\n");
-#endif
  
  	return -ENODEV;

  }
@@ -91,18 +87,14 @@ static int spl_mmc_find_device(struct mmc **mmcp, u32 
boot_device)
err = mmc_initialize(NULL);
  #endif /* DM_MMC */
if (err) {
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
printf("spl: could not initialize mmc. error: %d\n", err);
-#endif
return err;
}
*mmcp = find_mmc_device(mmc_dev);
err = *mmcp ? 0 : -ENODEV;
if (err) {
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
printf("spl: could not find mmc device %d. error: %d\n",
   mmc_dev, err);
-#endif
return err;
}
  
@@ -135,10 +127,8 @@ static int mmc_load_image_raw_partition(struct spl_image_info *spl_image,
  
  	err = part_get_info(mmc_get_blk_desc(mmc), partition, &info);

if (err) {
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
puts("spl: partition error\n");
-#endif
-   return -1;
+   return -err;
}
  
  #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR

@@ -164,9 +154,7 @@ static int mmc_load_image_raw_os(struct spl_image_info 
*spl_image,
CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS,
(void *)CONFIG_SPL_PAYLOAD_ARGS_ADDR);
if (count != CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS) {
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
puts("mmc_load_image_raw_os: mmc block read error\n");
-#endif
return -1;
}
  #endif/* CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTOR */
@@ -368,9 +356,7 @@ int spl_mmc_load(struct spl_image_info *spl_image,
err = mmc_init(mmc);
if (err) {
mmc = NULL;
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
printf("spl: mmc init failed with error: %d\n", err);
-#endif
return err;
}
}
@@ -387,9 +373,7 @@ int spl_mmc_load(struct spl_image_info *spl_image,
err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
  
  		if (err) {

-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
puts("spl: mmc partition switch failed\n");
-#endif
return err;
}
/* Fall through */
@@ -428,10 +412,8 @@ int spl_mmc_load(struct spl_image_info *spl_image,
  
  		break;

  #endif
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
default:
    puts("spl: mmc: wrong boot mode\n");
-#endif
}
  
  	return err;


Reviewed-by: Sean Anderson 

Re: [PATCH 07/14] log: Avoid including function names by default

[Sean Anderson]

On 7/20/24 02:16, Simon Glass wrote:

Unless function names are requested, the logging system should not
compile these into the code. Adjust the macros to handle this.

Enable CONFIG_LOGF_FUNC logging for sandbox since the tests expect the
function names to be included. Fix up the pinmux test which checks a
logging statement.

Signed-off-by: Simon Glass 
---

  common/log_console.c  |  4 ++--
  configs/sandbox_defconfig |  1 +
  include/log.h | 18 --
  test/cmd/pinmux.c |  8 +++-
  test/log/log_test.c   |  4 ++--
  5 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/common/log_console.c b/common/log_console.c
index c27101b8fe2..9376baad664 100644
--- a/common/log_console.c
+++ b/common/log_console.c
@@ -38,10 +38,10 @@ static int log_console_emit(struct log_device *ldev, struct 
log_rec *rec)
printf("%d-", rec->line);
if (fmt & BIT(LOGF_FUNC)) {
if (CONFIG_IS_ENABLED(USE_TINY_PRINTF)) {
-   printf("%s()", rec->func);
+   printf("%s()", rec->func ?: "?");
} else {
printf("%*s()", CONFIG_LOGF_FUNC_PAD,
-  rec->func);
+  rec->func ?: "?");
}
}
}
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index e2db66d4a25..d43d92d0cf2 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -38,6 +38,7 @@ CONFIG_PRE_CONSOLE_BUFFER=y
  CONFIG_LOG=y
  CONFIG_LOG_MAX_LEVEL=9
  CONFIG_LOG_DEFAULT_LEVEL=6
+CONFIG_LOGF_FUNC=y
  CONFIG_DISPLAY_BOARDINFO_LATE=y
  CONFIG_STACKPROTECTOR=y
  CONFIG_ANDROID_AB=y
diff --git a/include/log.h b/include/log.h
index fc0d5984472..67b6fd70050 100644
--- a/include/log.h
+++ b/include/log.h
@@ -125,7 +125,7 @@ static inline int log_uc_cat(enum uclass_id id)
   * @level: Level of log record (indicating its severity)
   * @file: File name of file where log record was generated
   * @line: Line number in file where log record was generated
- * @func: Function where log record was generated
+ * @func: Function where log record was generated, NULL if not known
   * @fmt: printf() format string for log record
   * @...: Optional parameters, according to the format string @fmt
   * Return: 0 if log record was emitted, -ve on error
@@ -141,7 +141,7 @@ int _log(enum log_category_t cat, enum log_level_t level, 
const char *file,
   * @level: Level of log record (indicating its severity)
   * @file: File name of file where log record was generated
   * @line: Line number in file where log record was generated
- * @func: Function where log record was generated
+ * @func: Function where log record was generated, NULL if not known
   * @addr: Starting address to display at start of line
   * @data: pointer to data buffer
   * @width:data value width.  May be 1, 2, or 4.
@@ -193,6 +193,12 @@ int _log_buffer(enum log_category_t cat, enum log_level_t 
level,
  #define _LOG_DEBUG0
  #endif
  
+#ifdef CONFIG_LOGF_FUNC

+#define _log_func  __func__
+#else
+#define _log_func  NULL
+#endif
+
  #if CONFIG_IS_ENABLED(LOG)
  
  /* Emit a log record if the level is less that the maximum */

@@ -201,7 +207,7 @@ int _log_buffer(enum log_category_t cat, enum log_level_t 
level,
if (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL) \
_log((enum log_category_t)(_cat), \
 (enum log_level_t)(_l | _LOG_DEBUG), __FILE__, \
-__LINE__, __func__, \
+__LINE__, _log_func, \
  pr_fmt(_fmt), ##_args); \
})
  
@@ -211,7 +217,7 @@ int _log_buffer(enum log_category_t cat, enum log_level_t level,

if (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL) \
_log_buffer((enum log_category_t)(_cat), \
(enum log_level_t)(_l | _LOG_DEBUG), __FILE__, \
-   __LINE__, __func__, _addr, _data, \
+   __LINE__, _log_func, _addr, _data, \
_width, _count, _linelen); \
})
  #else
@@ -302,7 +308,7 @@ void __assert_fail(const char *assertion, const char *file, 
unsigned int line,
   */
  #define assert(x) \
({ if (!(x) && _DEBUG) \
-   __assert_fail(#x, __FILE__, __LINE__, __func__); })
+   __assert_fail(#x, __FILE__, __LINE__, _log_func); })


Can we keep this one in? It's only compiled for debug and we are including 
file/line anyway.


  /*
   * This one actually gets compiled in even without DEBUG. It doesn't include 
the
@@ -314,7 +320,7 @@ void __assert_fail(const char *assertion, const char *file, 
unsigned int line,
  #define assert_noisy(x) \
({ bool _val = (x); \
if (!_val) \
-   __assert_fail(#x, "?", __LINE__, __func__); \
+   __assert_fail

Re: [PATCH 06/14] mmc: Use logging instead of pr_err()

[Sean Anderson]

ort enhanced attribute\n");
+   log_err("Card does not support enhanced attribute\n");
return -EMEDIUMTYPE;
}
  
@@ -1171,8 +1170,8 @@ int mmc_hwpart_config(struct mmc *mmc,

(ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT+1] << 8) +
ext_csd[EXT_CSD_MAX_ENH_SIZE_MULT];
if (tot_enh_size_mult > max_enh_size_mult) {
-   pr_err("Total enhanced size exceeds maximum (%u > %u)\n",
-  tot_enh_size_mult, max_enh_size_mult);
+   log_err("Total enhanced size exceeds maximum (%x > %x)\n",
+   tot_enh_size_mult, max_enh_size_mult);
return -EMEDIUMTYPE;
}
  
@@ -1205,7 +1204,7 @@ int mmc_hwpart_config(struct mmc *mmc,
  
  	if (ext_csd[EXT_CSD_PARTITION_SETTING] &

EXT_CSD_PARTITION_SETTING_COMPLETED) {
-   pr_err("Card already partitioned\n");
+   log_err("Card already partitioned\n");
return -EPERM;
}
  
@@ -1876,7 +1875,7 @@ error:

}
}
  
-	pr_err("unable to select a mode\n");

+   log_err("unable to select a mode\n");
return -ENOTSUPP;
  }
  
@@ -2254,7 +2253,7 @@ error:

}
}
  
-	pr_err("unable to select a mode : %d\n", err);

+   log_err("unable to select a mode: %d\n", err);
  
  	return -ENOTSUPP;

  }
@@ -2921,8 +2920,10 @@ retry:
  
  		if (err) {

  #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-   if (!quiet)
-   pr_err("Card did not respond to voltage select! : 
%d\n", err);
+   if (!quiet) {
+   log_err("Card did not respond to voltage select! : 
%d\n",
+   err);
+   }
  #endif
return -EOPNOTSUPP;
}
@@ -2955,7 +2956,7 @@ int mmc_start_init(struct mmc *mmc)
   | MMC_CAP(MMC_LEGACY) |
   MMC_MODE_1BIT);
} else {
-   pr_err("bus_mode requested is not supported\n");
+   log_err("bus_mode requested is not 
supported\n");
return -EINVAL;
}
}
@@ -2975,7 +2976,7 @@ int mmc_start_init(struct mmc *mmc)
if (no_card) {
mmc->has_init = 0;
  #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
-       pr_err("MMC: no card present\n");
+   log_err("MMC: no card present\n");
  #endif
return -ENOMEDIUM;
}
@@ -3104,7 +3105,7 @@ static int mmc_probe(struct bd_info *bis)
uclass_foreach_dev(dev, uc) {
ret = device_probe(dev);
if (ret)
-   pr_err("%s - probe failed: %d\n", dev->name, ret);
+   log_err("%s - probe failed: %d\n", dev->name, ret);
}
  
  	return 0;


Reviewed-by: Sean Anderson 

Re: [PATCH 05/14] mmc: Use logging instead of printf()

[Sean Anderson]

On 7/20/24 02:16, Simon Glass wrote:

The code makes quite a few uses of __func__ which puts the function
name into the resulting SPL image. Use the log subsystem instead, to
reduce size.

The CONFIG_LOGF_FUNC option can be used to enable the function name.

Signed-off-by: Simon Glass 
---

  drivers/mmc/sdhci.c | 46 +++--
  1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 560b7e889c7..24998233543 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -32,8 +32,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
sdhci_writeb(host, mask, SDHCI_SOFTWARE_RESET);
while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
if (timeout == 0) {
-   printf("%s: Reset 0x%x never completed.\n",
-  __func__, (int)mask);
+   log_warning("Reset %x never completed\n", (int)mask);


I think this cast can be removed too


return;
}
timeout--;
@@ -139,8 +138,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, 
struct mmc_data *data)
do {
stat = sdhci_readl(host, SDHCI_INT_STATUS);
if (stat & SDHCI_INT_ERROR) {
-   pr_debug("%s: Error detected in status(0x%X)!\n",
-__func__, stat);
+   log_debug("Error detected in status(%x)!\n", stat);
return -EIO;
}
if (!transfer_done && (stat & rdy)) {
@@ -173,7 +171,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, 
struct mmc_data *data)
if (timeout-- > 0)
udelay(10);
else {
-   printf("%s: Transfer data timeout\n", __func__);
+   log_err("Transfer data timeout\n");
return -ETIMEDOUT;
}
} while (!(stat & SDHCI_INT_DATA_END));
@@ -232,7 +230,7 @@ static int sdhci_send_command(struct mmc *mmc, struct 
mmc_cmd *cmd,
  
  	while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {

if (time >= cmd_timeout) {
-   printf("%s: MMC: %d busy ", __func__, mmc_dev);
+   log_warning("MMC: %d busy ", mmc_dev);


Maybe this should be "mmc%d busy\n"


if (2 * cmd_timeout <= SDHCI_CMD_MAX_TIMEOUT) {
cmd_timeout += cmd_timeout;
printf("timeout increasing to: %u ms.\n",
@@ -316,8 +314,8 @@ static int sdhci_send_command(struct mmc *mmc, struct 
mmc_cmd *cmd,
}
  
  		if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) {

-   printf("%s: Timeout for status update: %08x %08x\n",
-  __func__, stat, mask);
+   log_warning("Timeout for status update: %08x %08x\n",
+   stat, mask);
return -ETIMEDOUT;
}
} while ((stat & mask) != mask);
@@ -358,7 +356,7 @@ static int sdhci_execute_tuning(struct udevice *dev, uint 
opcode)
struct mmc *mmc = mmc_get_mmc_dev(dev);
struct sdhci_host *host = mmc->priv;
  
-	debug("%s\n", __func__);

+   log_debug("tuning\n");


I think this one should be left as-is. Or at least given something more 
descriptive.

  
  	if (host->ops && host->ops->platform_execute_tuning) {

err = host->ops->platform_execute_tuning(mmc, opcode);
@@ -380,8 +378,7 @@ int sdhci_set_clock(struct mmc *mmc, unsigned int clock)
while (sdhci_readl(host, SDHCI_PRESENT_STATE) &
   (SDHCI_CMD_INHIBIT | SDHCI_DATA_INHIBIT)) {
if (timeout == 0) {
-   printf("%s: Timeout to wait cmd & data inhibit\n",
-  __func__);
+   log_err("Timeout to wait cmd & data inhibit\n");


Might as well fix the grammar while we're at it

log_err("Timeout waiting for cmd & data inhibit\n");


return -EBUSY;
}
  
@@ -397,7 +394,7 @@ int sdhci_set_clock(struct mmc *mmc, unsigned int clock)

if (host->ops && host->ops->set_delay) {
ret = host->ops->set_delay(host);
if (ret) {
-   printf("%s: Error while setting tap delay\n", __func__);
+   log_err("Error while setting tap delay\n");
return ret;
}
}
@@ -405,7 +402,7 @@ int sdhci_set_clock(struct mmc *mmc, unsigned int clock)
if (host->ops && host->ops->config_dll) {
ret = host->ops->config_dll(host, clock, false);
if (ret) {
-   printf("%s: Error while configuring dll\n", __func__);
+   log_err("Error con

Re: [PATCH 04/14] spl: Remove remaining #ifdef in spl_parse_image_header()

[Sean Anderson]

On 7/20/24 02:16, Simon Glass wrote:

Define spl_set_header_raw_uboot() always so we can drop the last #ifdef
in this function.

Signed-off-by: Simon Glass 
---

  common/spl/spl.c | 15 ---
  1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 02567e766f1..745efdd9b28 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -245,7 +245,6 @@ __weak struct legacy_img_hdr *spl_get_load_buffer(ssize_t 
offset, size_t size)
return map_sysmem(CONFIG_TEXT_BASE + offset, 0);
  }
  
-#ifdef CONFIG_SPL_RAW_IMAGE_SUPPORT

  void spl_set_header_raw_uboot(struct spl_image_info *spl_image)
  {
ulong u_boot_pos = spl_get_image_pos();
@@ -273,7 +272,6 @@ void spl_set_header_raw_uboot(struct spl_image_info 
*spl_image)
spl_image->os = IH_OS_U_BOOT;
spl_image->name = "U-Boot";
  }
-#endif
  
  __weak int spl_parse_board_header(struct spl_image_info *spl_image,

  const struct spl_boot_device *bootdev,
@@ -357,16 +355,19 @@ int spl_parse_image_header(struct spl_image_info 
*spl_image,
sizeof(*header)))
return 0;
  
-#ifdef CONFIG_SPL_RAW_IMAGE_SUPPORT

+   if (IS_ENABLED(CONFIG_SPL_RAW_IMAGE_SUPPORT)) {
/* Signature not found - assume u-boot.bin */
debug("mkimage signature not found - ih_magic = %x\n",
-   header->ih_magic);
+ header->ih_magic);
spl_set_header_raw_uboot(spl_image);
-#else
-   /* RAW image not supported, proceed to other boot methods. */
+   } else {
+   /*
+* RAW image not supported, proceed to other boot
+* methods
+*/


nit: leave this comment as-is, since it is under 80 columns


debug("Raw boot image support not enabled, proceeding to other boot 
methods\n");
return -EINVAL;
-#endif
+   }
  
  	return 0;

  }


Reviewed-by: Sean Anderson 

Re: [PATCH 03/14] spl: Remove some #ifdefs in spl_parse_image_header()

[Sean Anderson]

On 7/20/24 02:16, Simon Glass wrote:

This function has a number of unnecessary #ifdefs so remove them.

Signed-off-by: Simon Glass 
---

  common/spl/spl.c | 22 +++---
  1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 6f4a8bfb3f4..02567e766f1 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -308,8 +308,10 @@ int spl_parse_image_header(struct spl_image_info 
*spl_image,
ret = spl_parse_legacy_header(spl_image, header);
if (ret)
return ret;
-   } else {
-#ifdef CONFIG_SPL_PANIC_ON_RAW_IMAGE
+   return 0;
+   }
+
+   if (IS_ENABLED(CONFIG_SPL_PANIC_ON_RAW_IMAGE)) {
/*
 * CONFIG_SPL_PANIC_ON_RAW_IMAGE is defined when the
 * code which loads images in SPL cannot guarantee that
@@ -319,10 +321,9 @@ int spl_parse_image_header(struct spl_image_info 
*spl_image,
 * is bad, and thus should be skipped silently.
 */
panic("** no mkimage signature but raw image not supported");
-#endif
+   }
  
-#if CONFIG_IS_ENABLED(OS_BOOT)

-#if defined(CONFIG_CMD_BOOTI)
+   if (CONFIG_IS_ENABLED(OS_BOOT) && IS_ENABLED(CONFIG_CMD_BOOTI)) {


Maybe this should be IS_ENABLED(CONFIG_SPL_OS_BOOT) since this file cannot be 
compiled except in SPL.


ulong start, size;
  
  		if (!booti_setup((ulong)header, &start, &size, 0)) {

@@ -336,7 +337,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
  spl_image->load_addr, spl_image->size);
return 0;
}
-#elif defined(CONFIG_CMD_BOOTZ)
+   } else if (CONFIG_IS_ENABLED(OS_BOOT) && IS_ENABLED(CONFIG_CMD_BOOTZ)) {
ulong start, end;
  
  		if (!bootz_setup((ulong)header, &start, &end)) {

@@ -350,11 +351,11 @@ int spl_parse_image_header(struct spl_image_info 
*spl_image,
  spl_image->load_addr, spl_image->size);
return 0;
}
-#endif
-#endif
+   }
  
-		if (!spl_parse_board_header(spl_image, bootdev, (const void *)header, sizeof(*header)))

-   return 0;
+   if (!spl_parse_board_header(spl_image, bootdev, (const void *)header,
+   sizeof(*header)))
+   return 0;
  
  #ifdef CONFIG_SPL_RAW_IMAGE_SUPPORT

/* Signature not found - assume u-boot.bin */
@@ -366,7 +367,6 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
debug("Raw boot image support not enabled, proceeding to other boot 
methods\n");
return -EINVAL;
  #endif
-   }
  
  	return 0;

  }


Reviewed-by: Sean Anderson 

Re: [PATCH 02/14] spl: Correct use of CMD_BOOTI and CMD_BOOTZ

[Sean Anderson]

On 7/20/24 02:16, Simon Glass wrote:

These should have a CONFIG_ prefix. Add it.

Signed-off-by: Simon Glass 
Fixes: 7a0d88076b9 ("Add in the ability to load and boot an uncompr...")
---

  common/spl/spl.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 7794ddccade..6f4a8bfb3f4 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -322,7 +322,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
  #endif
  
  #if CONFIG_IS_ENABLED(OS_BOOT)

-#if defined(CMD_BOOTI)
+#if defined(CONFIG_CMD_BOOTI)
ulong start, size;
  
  		if (!booti_setup((ulong)header, &start, &size, 0)) {

@@ -336,7 +336,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
  spl_image->load_addr, spl_image->size);
return 0;
}
-#elif defined(CMD_BOOTZ)
+#elif defined(CONFIG_CMD_BOOTZ)
ulong start, end;
  
  		if (!bootz_setup((ulong)header, &start, &end)) {


Reviewed-by: Sean Anderson 

Re: [PATCH v2 15/21] spl: Plumb in the Universal Payload handoff

[Sean Anderson]

On 7/20/24 08:36, Simon Glass wrote:

Hi Sean,

On Thu, 18 Jul 2024 at 14:54, Sean Anderson  wrote:


On 7/13/24 03:00, Simon Glass wrote:

Specify the FIT and include information about each loaded image, as
required by the UPL handoff.

Write the UPL handoff into the bloblist before jumping to the next phase.

Control this using a runtime flag to avoid conflicting with other
handoff mechanisms.

Signed-off-by: Simon Glass 
---

Changes in v2:
- Hang when something goes wrong, to avoid a broken boot
- Add a runtime flag to enable UPL

   common/spl/spl.c  |  8 
   common/spl/spl_fit.c  | 22 ++
   include/asm-generic/global_data.h |  4 
   3 files changed, 34 insertions(+)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 7794ddccade..d6a364de6ee 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -810,6 +810,14 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
   printf(SPL_TPL_PROMPT
  "SPL hand-off write failed (err=%d)\n", ret);
   }
+ if (CONFIG_IS_ENABLED(UPL_OUT) && (gd->flags & GD_FLG_UPL)) {
+ ret = spl_write_upl_handoff(&spl_image);
+ if (ret) {
+ printf(SPL_TPL_PROMPT
+"UPL hand-off write failed (err=%d)\n", ret);
+ hang();
+ }
+ }
   if (CONFIG_IS_ENABLED(BLOBLIST)) {
   ret = bloblist_finish();
   if (ret)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 2a097f4464c..b288f675ae3 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -12,6 +12,7 @@
   #include 
   #include 
   #include 
+#include 
   #include 
   #include 
   #include 
@@ -645,6 +646,8 @@ static int spl_fit_load_fpga(struct spl_fit_info *ctx,
   printf("%s: Cannot load the FPGA: %i\n", __func__, ret);
   return ret;
   }
+ upl_add_image(node, fpga_image.load_addr, fpga_image.size,
+   fdt_getprop(ctx->fit, node, FIT_DESC_PROP, NULL));


Does load_addr even make sense for FPGAs?


Well the images do get loaded into RAM at a particular address.


No they don't. I mean, technically yes, but it's just an intermediate step
before being programmed into the FPGA. Much like how executable boot images
may be loaded to an intermediate address before being copied to their final
location. And the load_addr is set to 0 a few lines above.

But don't bother special-casing this if you do the next part.



And I noticed that you always call this after load_simple_fit/fit_image_load.
Could we call upl_add_image in those functions instead?


Yes, although it means compiling upl.h for the host. Perhaps it isn't
that bad though and it avoids repeating code.


I would prefer that to help ensure that future programmers don't forget it.




   return spl_fit_upload_fpga(ctx, node, &fpga_image);
   }
@@ -768,6 +771,9 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
   if (ret)
   return ret;

+ upl_add_image(node, spl_image->load_addr, spl_image->size,
+   fdt_getprop(ctx.fit, node, FIT_DESC_PROP, NULL));
+
   /*
* For backward compatibility, we treat the first node that is
* as a U-Boot image, if no OS-type has been declared.
@@ -811,6 +817,8 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
  __func__, index, ret);
   return ret;
   }
+ upl_add_image(node, image_info.load_addr, image_info.size,
+   fdt_getprop(ctx.fit, node, FIT_DESC_PROP, NULL));

   if (spl_fit_image_is_fpga(ctx.fit, node))
   spl_fit_upload_fpga(&ctx, node, &image_info);
@@ -847,6 +855,8 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
   spl_image->entry_point = spl_image->load_addr;

   spl_image->flags |= SPL_FIT_FOUND;
+ upl_set_fit_info(map_to_sysmem(ctx.fit), ctx.conf_node,
+  spl_image->entry_point);


I think this should be virt_to_phys, since we aren't really mapping it.


We want to pass the address, which (for sandbox) has to be able to be
mapped back to a pointer. Everywhere else in sandbox this is how we
handle that...


This is really a nit. But I thought that map_to_sysmem implied a pairing with
unmap. But I guess that's map_physmem.

--Sean

Re: [PATCH v2 04/21] sandbox: Enable SPL_LOAD_BLOCK

[Sean Anderson]

On 7/18/24 09:28, Sean Anderson wrote:

On 7/13/24 03:00, Simon Glass wrote:

Enable this option for all sandbox boards so that the FIT-loading code
can be used without generating an error about block devices not being
supported.

Signed-off-by: Simon Glass 
---

(no changes since v1)

  common/spl/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index af43b5f5d3c..970a4a346e4 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -288,6 +288,7 @@ config SPL_BOARD_INIT
  config SPL_LOAD_BLOCK
  bool
+    default y if SANDBOX
  help
    Support loading images from block devices. This adds a bl_len member
    to struct spl_load_info.


NACK.

Enable one of the many configs which select this one instead.

--Sean


OK, so I think you wanted to add this because of the previous patch/patch 20.

I think a better solution would be to just reduce the bl_len to 1, since unix 
I/O
is byte-oriented anyway and we now have several tests for bl_len != 1.

--Sean

Re: [PATCH v2 20/21] sandbox: Add an SPL loader for UPL

[Sean Anderson]

On 7/13/24 03:00, Simon Glass wrote:

Add support for loading a UPL image from SPL. This uses the simple FIT
implementation, but also loads the full FIT just to permit more testing.

Signed-off-by: Simon Glass 
---

(no changes since v1)

  arch/sandbox/cpu/spl.c | 49 +-
  arch/sandbox/include/asm/spl.h |  1 +
  2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c
index 39869f27a7b..f665f86d64c 100644
--- a/arch/sandbox/cpu/spl.c
+++ b/arch/sandbox/cpu/spl.c
@@ -3,14 +3,18 @@
   * Copyright (c) 2016 Google, Inc
   */
  
+#define LOG_CATEGORY	LOGC_BOOT

+
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -52,7 +56,8 @@ void board_init_f(ulong flag)
  void board_boot_order(u32 *spl_boot_list)
  {
spl_boot_list[0] = BOOT_DEVICE_VBE;
-   spl_boot_list[1] = BOOT_DEVICE_BOARD;
+   spl_boot_list[1] = BOOT_DEVICE_UPL;
+   spl_boot_list[2] = BOOT_DEVICE_BOARD;
  }
  
  static int spl_board_load_file(struct spl_image_info *spl_image,

@@ -246,3 +251,45 @@ int sandbox_spl_load_fit(char *fname, int maxlen, struct 
spl_image_info *image)
  
  	return 0;

  }
+
+static int upl_load_from_image(struct spl_image_info *spl_image,
+  struct spl_boot_device *bootdev)
+{
+   long long size;
+   char *fname;
+   int ret, fd;
+   ulong addr;
+
+   if (!CONFIG_IS_ENABLED(UPL_OUT))
+   return -ENOTSUPP;
+
+   spl_upl_init();
+   fname = os_malloc(256);
+
+   ret = sandbox_spl_load_fit(fname, 256, spl_image);
+   if (ret)
+   return log_msg_ret("fit", ret);
+   spl_image->flags = SPL_SANDBOXF_ARG_IS_BUF;
+   spl_image->arg = map_sysmem(spl_image->load_addr, 0);
+   /* size is set by load_simple_fit(), offset is left as 0 */
+
+   /* now read the whole FIT into memory */


Why do we have to do this? Didn't we just load the FIT in sandbox_spl_load_fit?


+   fd = os_open(fname, OS_O_RDONLY);
+   if (fd < 0)
+   return log_msg_ret("op2", -ENOENT);
+   if (os_get_filesize(fname,  &size))
+   return log_msg_ret("fis", -ENOENT);
+
+   /* place it after the loaded image, allowing plenty of space */
+   addr = ALIGN(spl_image->load_addr + size, 0x1000);
+   log_debug("Loading whole FIT to %lx\n", addr);
+   if (os_read(fd, map_sysmem(addr, 0), size) != size)
+   return log_msg_ret("rea", -EIO);
+   os_close(fd);
+
+   /* tell UPL where it is */
+   upl_set_fit_addr(addr);
+
+   return 0;
+}
+SPL_LOAD_IMAGE_METHOD("upl", 4, BOOT_DEVICE_UPL, upl_load_from_image);
diff --git a/arch/sandbox/include/asm/spl.h b/arch/sandbox/include/asm/spl.h
index d50d9ad6b48..d824b2123a2 100644
--- a/arch/sandbox/include/asm/spl.h
+++ b/arch/sandbox/include/asm/spl.h
@@ -18,6 +18,7 @@ enum {
BOOT_DEVICE_NOR,
BOOT_DEVICE_SPI,
BOOT_DEVICE_NAND,
+   BOOT_DEVICE_UPL,
  };
  
  /**

Re: [PATCH v2 03/21] test: Move some SPL-loading test-code into sandbox common

[Sean Anderson]

On 7/13/24 03:00, Simon Glass wrote:

This code is useful for loading an image in sandbox_spl so move it into
a place where it can be called as needed.

Signed-off-by: Simon Glass 
---

(no changes since v1)

  arch/sandbox/cpu/spl.c | 67 ++
  arch/sandbox/include/asm/spl.h | 14 +++
  test/image/spl_load_os.c   | 53 +--
  3 files changed, 82 insertions(+), 52 deletions(-)

diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c
index 9ad9da686c6..caa8e0b3b01 100644
--- a/arch/sandbox/cpu/spl.c
+++ b/arch/sandbox/cpu/spl.c
@@ -6,6 +6,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -179,3 +180,69 @@ int handoff_arch_save(struct spl_handoff *ho)
  
  	return 0;

  }
+
+/* Context used to hold file descriptor */
+struct load_ctx {
+   int fd;
+};
+
+static ulong read_fit_image(struct spl_load_info *load, ulong offset,
+   ulong size, void *buf)
+{
+   struct load_ctx *load_ctx = load->priv;
+   off_t ret;
+   ssize_t res;
+
+   ret = os_lseek(load_ctx->fd, offset, OS_SEEK_SET);
+   if (ret < 0) {
+   printf("Failed to seek to %zx, got %zx (errno=%d)\n", offset,
+  ret, errno);
+   return log_msg_ret("lse", ret);
+   }
+
+   res = os_read(load_ctx->fd, buf, size);
+   if (res < 0) {
+   printf("Failed to read %lx bytes, got %ld (errno=%d)\n",
+  size, res, errno);
+   return log_msg_ret("osr", res);
+   }
+
+   return size;
+}
+
+int sandbox_spl_load_fit(char *fname, int maxlen, struct spl_image_info *image)
+{
+   struct legacy_img_hdr *header;
+   struct load_ctx load_ctx;
+   struct spl_load_info load;
+   int ret;
+   int fd;
+
+   memset(&load, '\0', sizeof(load));
+   spl_set_bl_len(&load, 512);
+   load.read = read_fit_image;
+
+   ret = sandbox_find_next_phase(fname, maxlen, true);
+   if (ret) {
+   printf("%s not found, error %d\n", fname, ret);
+   return log_msg_ret("nph", ret);
+   }
+
+   header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
+
+   log_debug("reading from %s\n", fname);
+   fd = os_open(fname, OS_O_RDONLY);
+   if (fd < 0)
+   return log_msg_ret("ope", -ENOENT);


should return errno

and I think longer messages won't hurt (this is sandbox after all).


+   if (os_read(fd, header, 512) != 512)


Actually, this should probably be sizeof(*header) and not 512


+   return log_msg_ret("rea", -EIO);


ditto errno/message


+   load_ctx.fd = fd;
+
+   load.priv = &load_ctx;
+
+   ret = spl_load_simple_fit(image, &load, 0, header);
+   if (ret)
+   return log_msg_ret("slf", ret);
+
+   return 0;
+}
diff --git a/arch/sandbox/include/asm/spl.h b/arch/sandbox/include/asm/spl.h
index 4fab24cd156..d50d9ad6b48 100644
--- a/arch/sandbox/include/asm/spl.h
+++ b/arch/sandbox/include/asm/spl.h
@@ -6,6 +6,8 @@
  #ifndef __asm_spl_h
  #define __asm_spl_h
  
+struct spl_image_info;

+
  enum {
BOOT_DEVICE_MMC1,
BOOT_DEVICE_MMC2,
@@ -31,4 +33,16 @@ enum {
   */
  int sandbox_find_next_phase(char *fname, int maxlen, bool use_img);
  
+/**

+ * sandbox_spl_load_fit() - Load the next phase from a FIT
+ *
+ * Loads a FIT containing the next phase and sets it up for booting
+ *
+ * @fname: Returns filename loaded
+ * @maxlen: Maximum length for @fname including \0
+ * @image: Place to put SPL-image information
+ * Return: 0 if OK, -ve on error
+ */
+int sandbox_spl_load_fit(char *fname, int maxlen, struct spl_image_info 
*image);
+
  #endif
diff --git a/test/image/spl_load_os.c b/test/image/spl_load_os.c
index 7d5fb9b07e0..56105a59236 100644
--- a/test/image/spl_load_os.c
+++ b/test/image/spl_load_os.c
@@ -10,63 +10,12 @@
  #include 
  #include 
  
-/* Context used for this test */

-struct text_ctx {
-   int fd;
-};
-
-static ulong read_fit_image(struct spl_load_info *load, ulong offset,
-   ulong size, void *buf)
-{
-   struct text_ctx *text_ctx = load->priv;
-   off_t ret;
-   ssize_t res;
-
-   ret = os_lseek(text_ctx->fd, offset, OS_SEEK_SET);
-   if (ret != offset) {
-   printf("Failed to seek to %zx, got %zx (errno=%d)\n", offset,
-  ret, errno);
-   return 0;
-   }
-
-   res = os_read(text_ctx->fd, buf, size);
-   if (res == -1) {
-   printf("Failed to read %lx bytes, got %ld (errno=%d)\n",
-  size, res, errno);
-   return 0;
-   }
-
-   return size;
-}
-
  static int spl_test_load(struct unit_test_state *uts)
  {
struct spl_image_info image;
-   struct legacy_img_hdr *header;
-   struct text_ctx text_ctx;
-   struct spl_load_info load;
char fname[256];
-   int ret;
-   i

Re: [PATCH v2 15/21] spl: Plumb in the Universal Payload handoff

[Sean Anderson]

On 7/13/24 03:00, Simon Glass wrote:

Specify the FIT and include information about each loaded image, as
required by the UPL handoff.

Write the UPL handoff into the bloblist before jumping to the next phase.

Control this using a runtime flag to avoid conflicting with other
handoff mechanisms.

Signed-off-by: Simon Glass 
---

Changes in v2:
- Hang when something goes wrong, to avoid a broken boot
- Add a runtime flag to enable UPL

  common/spl/spl.c  |  8 
  common/spl/spl_fit.c  | 22 ++
  include/asm-generic/global_data.h |  4 
  3 files changed, 34 insertions(+)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 7794ddccade..d6a364de6ee 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -810,6 +810,14 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
printf(SPL_TPL_PROMPT
   "SPL hand-off write failed (err=%d)\n", ret);
}
+   if (CONFIG_IS_ENABLED(UPL_OUT) && (gd->flags & GD_FLG_UPL)) {
+   ret = spl_write_upl_handoff(&spl_image);
+   if (ret) {
+   printf(SPL_TPL_PROMPT
+  "UPL hand-off write failed (err=%d)\n", ret);
+   hang();
+   }
+   }
if (CONFIG_IS_ENABLED(BLOBLIST)) {
ret = bloblist_finish();
if (ret)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 2a097f4464c..b288f675ae3 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -12,6 +12,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -645,6 +646,8 @@ static int spl_fit_load_fpga(struct spl_fit_info *ctx,
printf("%s: Cannot load the FPGA: %i\n", __func__, ret);
return ret;
}
+   upl_add_image(node, fpga_image.load_addr, fpga_image.size,
+ fdt_getprop(ctx->fit, node, FIT_DESC_PROP, NULL));


Does load_addr even make sense for FPGAs?

And I noticed that you always call this after load_simple_fit/fit_image_load.
Could we call upl_add_image in those functions instead?


return spl_fit_upload_fpga(ctx, node, &fpga_image);
  }
@@ -768,6 +771,9 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
if (ret)
return ret;
  
+	upl_add_image(node, spl_image->load_addr, spl_image->size,

+ fdt_getprop(ctx.fit, node, FIT_DESC_PROP, NULL));
+
/*
 * For backward compatibility, we treat the first node that is
 * as a U-Boot image, if no OS-type has been declared.
@@ -811,6 +817,8 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
   __func__, index, ret);
return ret;
}
+   upl_add_image(node, image_info.load_addr, image_info.size,
+ fdt_getprop(ctx.fit, node, FIT_DESC_PROP, NULL));
  
  		if (spl_fit_image_is_fpga(ctx.fit, node))

spl_fit_upload_fpga(&ctx, node, &image_info);
@@ -847,6 +855,8 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
spl_image->entry_point = spl_image->load_addr;
  
  	spl_image->flags |= SPL_FIT_FOUND;

+   upl_set_fit_info(map_to_sysmem(ctx.fit), ctx.conf_node,
+spl_image->entry_point);


I think this should be virt_to_phys, since we aren't really mapping it.

  
  	return 0;

  }
@@ -891,6 +901,9 @@ int spl_load_fit_image(struct spl_image_info *spl_image,
if (ret < 0)
return ret;
  
+	upl_add_image(ret, fw_data, fw_len,

+ fdt_getprop((void *)header, ret, FIT_DESC_PROP, NULL));
+
spl_image->size = fw_len;
spl_image->load_addr = fw_data;
if (fit_image_get_entry(header, ret, &spl_image->entry_point))
@@ -911,6 +924,9 @@ int spl_load_fit_image(struct spl_image_info *spl_image,
if (ret >= 0) {
spl_image->fdt_addr = (void *)dt_data;
  
+		upl_add_image(ret, dt_data, dt_len,

+ fdt_getprop((void *)header, ret, FIT_DESC_PROP, NULL));
+
if (spl_image->os == IH_OS_U_BOOT) {
/* HACK: U-Boot expects FDT at a specific address */
fdt_hack = spl_image->load_addr + spl_image->size;
@@ -940,7 +956,13 @@ int spl_load_fit_image(struct spl_image_info *spl_image,
 &img_data, &img_len);
if (ret < 0)
return ret;
+   upl_add_image(ret, img_data, img_len,
+ fdt_getprop((void *)header, ret, FIT_DESC_PROP, NULL));
}
  
+	spl_image->flags |= SPL_FIT_FOUND;


This change is unrelated and should be moved into a separate commit.


+   upl_set_fit_info(map_to_sysmem(header), conf_noffset,
+spl_image->entry_point);
+
return 0;
  }
diff --git a/include/asm-generi

Re: [PATCH v2 04/21] sandbox: Enable SPL_LOAD_BLOCK

[Sean Anderson]

On 7/13/24 03:00, Simon Glass wrote:

Enable this option for all sandbox boards so that the FIT-loading code
can be used without generating an error about block devices not being
supported.

Signed-off-by: Simon Glass 
---

(no changes since v1)

  common/spl/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index af43b5f5d3c..970a4a346e4 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -288,6 +288,7 @@ config SPL_BOARD_INIT
  
  config SPL_LOAD_BLOCK

bool
+   default y if SANDBOX
help
  Support loading images from block devices. This adds a bl_len member
  to struct spl_load_info.


NACK.

Enable one of the many configs which select this one instead.

--Sean

Re: [PATCH] ext4: Improve feature checking

[Sean Anderson]

On 6/30/24 20:23, Sean Anderson wrote:

On 6/30/24 17:25, Richard Weinberger wrote:

Evaluate the filesystem incompat and ro_compat bit fields to judge
whether the filesystem can be read or written.
For the read side only a scary warning is shown so far.
I'd love to about mounting too, but I fear this will break some setups


I think you are missing a verb in the first half of your sentence


where the driver works by chance.

Signed-off-by: Richard Weinberger 
---
After facing ext4 write corruptions and read errors I checked
the ext4 implementation briefly.
Hopefully this patch helps other users to detect earlier that
they're using ext4 features which are not supported by u-boot.

To make feature checking possible I had to figure what this
implementation actually supports.
I hope I got them right, feedback is welcome!

Thanks,
//richard
---
  fs/ext4/ext4_common.c |  8 +++
  fs/ext4/ext4_write.c  | 12 --
  include/ext4fs.h  | 55 ++-
  3 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 2ff0dca249..7148d35ee0 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2386,6 +2386,14 @@ int ext4fs_mount(void)
  fs->inodesz = 128;
  fs->gdsize = 32;
  } else {
+    int missing = __le32_to_cpu(data->sblock.feature_incompat) & \
+  ~(EXT4_FEATURE_INCOMPAT_SUPP | \
+    EXT4_FEATURE_INCOMPAT_SUPP_LAZY_RO);
+
+    if (missing)
+    log_err("fs uses incompatible features: %08x, failures *are* 
expected!\n",
+    missing);
+


I think it is a bit unclear to the user what their action should be. Maybe 
something like

printf("EXT4 incompatible features: %08x, ignoring...\n")

and then maybe add a comment like what you have in the commit message.


or maybe even

printf("EXT4 incompatible features: %08x, mounting read-only...\n")


  debug("EXT4 features COMPAT: %08x INCOMPAT: %08x RO_COMPAT: %08x\n",
    __le32_to_cpu(data->sblock.feature_compatibility),
    __le32_to_cpu(data->sblock.feature_incompat),
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index d057f6b5a7..4aae3c5f7f 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -869,6 +869,7 @@ int ext4fs_write(const char *fname, const char *buffer,
  ALLOC_CACHE_ALIGN_BUFFER(char, filename, 256);
  bool store_link_in_inode = false;
  memset(filename, 0x00, 256);
+    int missing_feat;
  if (type != FILETYPE_REG && type != FILETYPE_SYMLINK)
  return -1;
@@ -882,8 +883,15 @@ int ext4fs_write(const char *fname, const char *buffer,
  return -1;
  }
-    if (le32_to_cpu(fs->sb->feature_ro_compat) & 
EXT4_FEATURE_RO_COMPAT_METADATA_CSUM) {
-    printf("Unsupported feature metadata_csum found, not writing.\n");
+    missing_feat = le32_to_cpu(fs->sb->feature_incompat) & 
~EXT4_FEATURE_INCOMPAT_SUPP;
+    if (missing_feat) {
+    log_err("Unsupported features found %08x, not writing.\n", 
missing_feat);
+    return -1;
+    }
+
+    missing_feat = le32_to_cpu(fs->sb->feature_ro_compat) & 
~EXT4_FEATURE_RO_COMPAT_SUPP;
+    if (missing_feat) {
+    log_err("Unsupported RO compat features found %08x, not writing.\n", 
missing_feat);
  return -1;
  }
diff --git a/include/ext4fs.h b/include/ext4fs.h
index d96edfd057..01b66f469f 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -34,12 +34,65 @@ struct disk_partition;
  #define EXT4_TOPDIR_FL    0x0002 /* Top of directory hierarchies*/
  #define EXT4_EXTENTS_FL    0x0008 /* Inode uses extents */
  #define EXT4_EXT_MAGIC    0xf30a
-#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM    0x0010
+
+#define EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER  0x0001
+#define EXT4_FEATURE_RO_COMPAT_LARGE_FILE    0x0002
+#define EXT4_FEATURE_RO_COMPAT_BTREE_DIR 0x0004
+#define EXT4_FEATURE_RO_COMPAT_HUGE_FILE 0x0008
+#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM  0x0010
+#define EXT4_FEATURE_RO_COMPAT_DIR_NLINK 0x0020
+#define EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE   0x0040
+#define EXT4_FEATURE_RO_COMPAT_QUOTA 0x0100
+#define EXT4_FEATURE_RO_COMPAT_BIGALLOC  0x0200
  #define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM 0x0400
+
+#define EXT4_FEATURE_INCOMPAT_FILETYPE  0x0002
+#define EXTE_FEATURE_INCOMPAT_RECOVER   0x0004
  #define EXT4_FEATURE_INCOMPAT_EXTENTS    0x0040
  #define EXT4_FEATURE_INCOMPAT_64BIT    0x0080
+#define EXT4_FEATURE_INCOMPAT_MMP   0x0100
+#define EXT4_FEATURE_INCOMPAT_FLEX_BG   0x0200
+#define EXT4_FEATURE_INCOMPAT_CSUM_SEED 0x2000
+#define EXT4_FEATURE_INCOMPAT_ENCRYPT   0x1
+
  #define EXT4_INDIRECT_BLOCKS    12
+/*
+ * Incompat features supported by this implementation.
+ */
+#define EXT4_FEATURE_INCOMPAT_SUPP ( EXT4_FEATU

Re: [PATCH] ext4: Improve feature checking

[Sean Anderson]

On 6/30/24 17:25, Richard Weinberger wrote:

Evaluate the filesystem incompat and ro_compat bit fields to judge
whether the filesystem can be read or written.
For the read side only a scary warning is shown so far.
I'd love to about mounting too, but I fear this will break some setups


I think you are missing a verb in the first half of your sentence


where the driver works by chance.

Signed-off-by: Richard Weinberger 
---
After facing ext4 write corruptions and read errors I checked
the ext4 implementation briefly.
Hopefully this patch helps other users to detect earlier that
they're using ext4 features which are not supported by u-boot.

To make feature checking possible I had to figure what this
implementation actually supports.
I hope I got them right, feedback is welcome!

Thanks,
//richard
---
  fs/ext4/ext4_common.c |  8 +++
  fs/ext4/ext4_write.c  | 12 --
  include/ext4fs.h  | 55 ++-
  3 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 2ff0dca249..7148d35ee0 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2386,6 +2386,14 @@ int ext4fs_mount(void)
fs->inodesz = 128;
fs->gdsize = 32;
} else {
+   int missing = __le32_to_cpu(data->sblock.feature_incompat) & \
+ ~(EXT4_FEATURE_INCOMPAT_SUPP | \
+   EXT4_FEATURE_INCOMPAT_SUPP_LAZY_RO);
+
+   if (missing)
+   log_err("fs uses incompatible features: %08x, failures *are* 
expected!\n",
+   missing);
+


I think it is a bit unclear to the user what their action should be. Maybe 
something like

printf("EXT4 incompatible features: %08x, ignoring...\n")
 
and then maybe add a comment like what you have in the commit message.



debug("EXT4 features COMPAT: %08x INCOMPAT: %08x RO_COMPAT: 
%08x\n",
  __le32_to_cpu(data->sblock.feature_compatibility),
  __le32_to_cpu(data->sblock.feature_incompat),
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index d057f6b5a7..4aae3c5f7f 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -869,6 +869,7 @@ int ext4fs_write(const char *fname, const char *buffer,
ALLOC_CACHE_ALIGN_BUFFER(char, filename, 256);
bool store_link_in_inode = false;
memset(filename, 0x00, 256);
+   int missing_feat;
  
  	if (type != FILETYPE_REG && type != FILETYPE_SYMLINK)

return -1;
@@ -882,8 +883,15 @@ int ext4fs_write(const char *fname, const char *buffer,
return -1;
}
  
-	if (le32_to_cpu(fs->sb->feature_ro_compat) & EXT4_FEATURE_RO_COMPAT_METADATA_CSUM) {

-   printf("Unsupported feature metadata_csum found, not 
writing.\n");
+   missing_feat = le32_to_cpu(fs->sb->feature_incompat) & 
~EXT4_FEATURE_INCOMPAT_SUPP;
+   if (missing_feat) {
+   log_err("Unsupported features found %08x, not writing.\n", 
missing_feat);
+   return -1;
+   }
+
+   missing_feat = le32_to_cpu(fs->sb->feature_ro_compat) & 
~EXT4_FEATURE_RO_COMPAT_SUPP;
+   if (missing_feat) {
+   log_err("Unsupported RO compat features found %08x, not 
writing.\n", missing_feat);
return -1;
}
  
diff --git a/include/ext4fs.h b/include/ext4fs.h

index d96edfd057..01b66f469f 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -34,12 +34,65 @@ struct disk_partition;
  #define EXT4_TOPDIR_FL0x0002 /* Top of directory 
hierarchies*/
  #define EXT4_EXTENTS_FL   0x0008 /* Inode uses extents */
  #define EXT4_EXT_MAGIC0xf30a
-#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM0x0010
+
+#define EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER  0x0001
+#define EXT4_FEATURE_RO_COMPAT_LARGE_FILE0x0002
+#define EXT4_FEATURE_RO_COMPAT_BTREE_DIR 0x0004
+#define EXT4_FEATURE_RO_COMPAT_HUGE_FILE 0x0008
+#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM  0x0010
+#define EXT4_FEATURE_RO_COMPAT_DIR_NLINK 0x0020
+#define EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE   0x0040
+#define EXT4_FEATURE_RO_COMPAT_QUOTA 0x0100
+#define EXT4_FEATURE_RO_COMPAT_BIGALLOC  0x0200
  #define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM 0x0400
+
+#define EXT4_FEATURE_INCOMPAT_FILETYPE  0x0002
+#define EXTE_FEATURE_INCOMPAT_RECOVER   0x0004
  #define EXT4_FEATURE_INCOMPAT_EXTENTS 0x0040
  #define EXT4_FEATURE_INCOMPAT_64BIT   0x0080
+#define EXT4_FEATURE_INCOMPAT_MMP   0x0100
+#define EXT4_FEATURE_INCOMPAT_FLEX_BG   0x0200
+#define EXT4_FEATURE_INCOMPAT_CSUM_SEED 0x2000
+#define EXT4_FEATURE_INCOMPAT_ENCRYPT   0x1
+
  #define EXT4_INDIRECT_BLOCKS  12
  
+/*

+ * Incompat features supported by this implementation.
+ */
+#define EXT4_FEATURE_INCOMPAT_SUPP ( EXT4_FEATURE_INCOMPAT_FILETYPE \
+

Re: silent u-boot not working

[Sean Anderson]

Hi Jonas,

On 6/13/24 09:16, Jonas Kvinge wrote:
> Hi,
> 
> I'm running openSUSE Tumbleweed on an RPI CM4, I'm trying to silence u-
> boot boot messages, with the default configuration there is an u-boot
> logo on the top right corner and boot text, I'd like to get rid of the
> logo and all text if possible.
> The keyboard is not working before the Linux kernel is loaded, so I can
> not interrupt u-boot and enter any commands, so where should I put "set
> env silent 1"?
> 
> I've followed the guide on
> 
> I have built u-boot from source using `make rpi_4_defconfig`
> configuration modified the configuration with make menuconfig, and
> verified this in .config before builing.
> CONFIG_SILENT_CONSOLE=y
> CONFIG_SILENT_U_BOOT_ONLY=y
> CONFIG_SILENT_CONSOLE_UPDATE_ON_SET=y
> CONFIG_SILENT_CONSOLE_UPDATE_ON_RELOC=y
> CONFIG_SILENT_CONSOLE_UNTIL_ENV=y
> 
> Copied the binary to /boot/efi/u-boot.bin and /boot/vc/u-boot.bin
> 
> I've also tried to define CONFIG_EXTRA_ENV_SETTINGS in
> include/configs/rpi.h according to:
>  and rebuilt and copied to binary again, still not working.
> 
> The text and logo is still present during boot, what am I doing wrong
> here?

I implemented a silent console recently. Here are the steps I took:

- Define CONFIG_DISABLE_CONSOLE
- set silent=y in the default environment (CONFIG_EXTRA_ENV_SETTINGS)

You can still enable the console by pressing any key at the (invisible) prompt.

Let me know if that works for you.

--Sean

Re: [PATCH 118/149] board: sipeed: Remove and add needed includes

[Sean Anderson]

On 4/30/24 22:42, Tom Rini wrote:

Remove  from this board vendor directory and when needed
add missing include files directly.

Signed-off-by: Tom Rini 
---
Cc: Sean Anderson 
---
  board/sipeed/maix/maix.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/sipeed/maix/maix.c b/board/sipeed/maix/maix.c
index 06653b5a8765..08077a1f9e13 100644
--- a/board/sipeed/maix/maix.c
+++ b/board/sipeed/maix/maix.c
@@ -3,7 +3,7 @@
   * Copyright (C) 2019-20 Sean Anderson 
   */
  
-#include 

+#include 
  #include 
  #include 
  #include 


Reviewed-by: Sean Anderson 

Re: [PATCH v2 01/23] clk: rockchip: rk356x: Add CLK_USB3OTGx_REF support

[Sean Anderson]

On 4/13/24 14:13, Jonas Karlman wrote:

The CLK_USB3OTGx_REF clocks is used as reference clock for USB3 block.

Add simple support to get rate of CLK_USB3OTGx_REF clocks to fix
reference clock period configuration.

Signed-off-by: Jonas Karlman 
---
v2: No change
---
  drivers/clk/rockchip/clk_rk3568.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/drivers/clk/rockchip/clk_rk3568.c 
b/drivers/clk/rockchip/clk_rk3568.c
index 57ef27dda893..999f48ea4b4e 100644
--- a/drivers/clk/rockchip/clk_rk3568.c
+++ b/drivers/clk/rockchip/clk_rk3568.c
@@ -2417,6 +2417,8 @@ static ulong rk3568_clk_get_rate(struct clk *clk)
case BCLK_EMMC:
rate = rk3568_emmc_get_bclk(priv);
break;
+   case CLK_USB3OTG0_REF:
+   case CLK_USB3OTG1_REF:
case TCLK_EMMC:
rate = OSC_HZ;
break;
@@ -2596,6 +2598,8 @@ static ulong rk3568_clk_set_rate(struct clk *clk, ulong 
rate)
case BCLK_EMMC:
ret = rk3568_emmc_set_bclk(priv, rate);
break;
+   case CLK_USB3OTG0_REF:
+   case CLK_USB3OTG1_REF:
case TCLK_EMMC:
ret = OSC_HZ;
break;


Acked-by: Sean Anderson 

Re: [PATCH v2 02/23] clk: rockchip: rk3588: Add REF_CLK_USB3OTGx support

[Sean Anderson]

On 4/13/24 14:13, Jonas Karlman wrote:

The REF_CLK_USB3OTGx clocks is used as reference clock for USB3 block.

Add simple support to get rate of REF_CLK_USB3OTGx clocks to fix
reference clock period configuration.

Signed-off-by: Jonas Karlman 
Reviewed-by: Quentin Schulz 
---
v2: Collect r-b tag
---
  drivers/clk/rockchip/clk_rk3588.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/drivers/clk/rockchip/clk_rk3588.c 
b/drivers/clk/rockchip/clk_rk3588.c
index 8f33843179b0..4c611a390499 100644
--- a/drivers/clk/rockchip/clk_rk3588.c
+++ b/drivers/clk/rockchip/clk_rk3588.c
@@ -1569,6 +1569,9 @@ static ulong rk3588_clk_get_rate(struct clk *clk)
case DCLK_DECOM:
rate = rk3588_mmc_get_clk(priv, clk->id);
break;
+   case REF_CLK_USB3OTG0:
+   case REF_CLK_USB3OTG1:
+   case REF_CLK_USB3OTG2:
case TMCLK_EMMC:
case TCLK_WDT0:
rate = OSC_HZ;
@@ -1734,6 +1737,9 @@ static ulong rk3588_clk_set_rate(struct clk *clk, ulong 
rate)
case DCLK_DECOM:
ret = rk3588_mmc_set_clk(priv, clk->id, rate);
break;
+   case REF_CLK_USB3OTG0:
+   case REF_CLK_USB3OTG1:
+   case REF_CLK_USB3OTG2:
case TMCLK_EMMC:
case TCLK_WDT0:
ret = OSC_HZ;


Acked-by: Sean Anderson 

[PATCH 3/3] patman: Add a tag for when a patch gets added to a series

[Sean Anderson]

When a patch is added to a series after the initial version, there are no
changes to note except that it is new. This is typically done to suppress
the "(no changes in vN)" message. It's also nice to add a change to the
cover letter so reviewers know there is an additional patch. Add a tag to
automate this process a bit.

There are two nits with the current approach:

- It favors '-' as a bullet point, but some people may prefer '*' (or
  something else)
- Tags (e.g. 'patman: ' in 'patman: foo bar') are not stripped. They are
  probably just noise in most series, but they may be useful for treewide
  series to distinguish 'gpio: frobnicate' from 'reset: frobnicate', so
  I've left them in.

Suggestions for the above appreciated.

Suggested-by: Douglas Anderson 
Signed-off-by: Sean Anderson 
---

 tools/patman/func_test.py   |  2 ++
 tools/patman/patchstream.py |  5 +
 tools/patman/patman.rst | 13 +
 ...t-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch |  1 +
 tools/patman/test/test01.txt|  1 +
 5 files changed, 22 insertions(+)

diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 3b4c9448882..af6c025a441 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -293,6 +293,7 @@ Changes in v4:
   change
 - Some changes
 - Some notes for the cover letter
+- fdt: Correct cast for sandbox in fdtdec_setup_mem_size_base()
 
 Simon Glass (2):
   pci: Correct cast for sandbox
@@ -342,6 +343,7 @@ Changes in v4:
 - Multi
   line
   change
+- New
 - Some changes
 
 Changes in v2:
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index ec1ca874fb2..a09ae9c7371 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -477,6 +477,11 @@ class PatchStream:
 self.change_version = self._parse_version(value, line)
 elif name == 'cc':
 self.commit.add_cc(value.split(','))
+elif name == 'added-in':
+version = self._parse_version(value, line)
+self.commit.add_change(version, '- New')
+self.series.AddChange(version, None, '- %s' %
+  self.commit.subject)
 else:
 self._add_warn('Line %d: Ignoring Commit-%s' %
(self.linenum, name))
diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
index 9971fa8c0fd..63b95a6b161 100644
--- a/tools/patman/patman.rst
+++ b/tools/patman/patman.rst
@@ -350,6 +350,19 @@ Cover-changes: n
 - This line will only appear in the cover letter
 
 
+Commit-added-in: n
+Add a change noting the version this commit was added in. This is
+equivalent to::
+
+Commit-changes: n
+- New
+
+Cover-changes: n
+- 
+
+It is a convenient shorthand for suppressing the '(no changes in vN)'
+message.
+
 Patch-cc / Commit-cc: Their Name 
 This copies a single patch to another email address. Note that the
 Cc: used by git send-email is ignored by patman, but will be
diff --git 
a/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch
 
b/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch
index 55a0d6756aa..48ea1793b47 100644
--- 
a/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch
+++ 
b/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch
@@ -23,6 +23,7 @@ Series-version: 3
 Patch-cc: fred
 Commit-cc: joe
 Series-process-log: sort, uniq
+Commit-added-in: 4
 Series-changes: 4
 - Some changes
 - Multi
diff --git a/tools/patman/test/test01.txt b/tools/patman/test/test01.txt
index 271d9bf043f..b2d73c5972c 100644
--- a/tools/patman/test/test01.txt
+++ b/tools/patman/test/test01.txt
@@ -51,6 +51,7 @@ Date:   Sat Apr 15 15:39:08 2017 -0600
 Patch-cc: fred
 Commit-cc: joe
 Series-process-log: sort, uniq
+Commit-added-in: 4
 Series-changes: 4
 - Some changes
 - Multi
-- 
2.37.1

[PATCH 2/3] patman: Add Commit-cc as an alias for Patch-cc

[Sean Anderson]

Most tags referring to commits (or patches) are named Commit-something. The
exception is Patch-cc. Add a Commit-cc alias so we can use whichever one is
convenient.

Signed-off-by: Sean Anderson 
---

 tools/patman/func_test.py| 5 -
 tools/patman/patchstream.py  | 2 ++
 tools/patman/patman.rst  | 2 +-
 ...dt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch | 1 +
 tools/patman/test/test01.txt | 1 +
 5 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 9c016fb5e9a..3b4c9448882 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -211,6 +211,7 @@ class TestFunctional(unittest.TestCase):
 'u-boot': ['u-boot@lists.denx.de'],
 'simon': [self.leb],
 'fred': [self.fred],
+'joe': [self.joe],
 }
 
 text = self._get_text('test01.txt')
@@ -259,6 +260,7 @@ class TestFunctional(unittest.TestCase):
 self.assertEqual('Postfix:\t  some-branch', next(lines))
 self.assertEqual('Cover: 4 lines', next(lines))
 self.assertEqual('  Cc:  %s' % self.fred, next(lines))
+self.assertEqual('  Cc:  %s' % self.joe, next(lines))
 self.assertEqual('  Cc:  %s' % self.leb,
  next(lines))
 self.assertEqual('  Cc:  %s' % mel, next(lines))
@@ -272,7 +274,8 @@ class TestFunctional(unittest.TestCase):
 
 self.assertEqual(('%s %s\0%s' % (args[0], rick, stefan)), cc_lines[0])
 self.assertEqual(
-'%s %s\0%s\0%s\0%s' % (args[1], self.fred, self.leb, rick, stefan),
+'%s %s\0%s\0%s\0%s\0%s' % (args[1], self.fred, self.joe, self.leb,
+   rick, stefan),
 cc_lines[1])
 
 expected = '''
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index e2e2a83e677..ec1ca874fb2 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -475,6 +475,8 @@ class PatchStream:
 elif name == 'changes':
 self.in_change = 'Commit'
 self.change_version = self._parse_version(value, line)
+elif name == 'cc':
+self.commit.add_cc(value.split(','))
 else:
 self._add_warn('Line %d: Ignoring Commit-%s' %
(self.linenum, name))
diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
index f4588c00fc1..9971fa8c0fd 100644
--- a/tools/patman/patman.rst
+++ b/tools/patman/patman.rst
@@ -350,7 +350,7 @@ Cover-changes: n
 - This line will only appear in the cover letter
 
 
-Patch-cc: Their Name 
+Patch-cc / Commit-cc: Their Name 
 This copies a single patch to another email address. Note that the
 Cc: used by git send-email is ignored by patman, but will be
 interpreted by git send-email if you use it.
diff --git 
a/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch
 
b/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch
index 56278a6ce9b..55a0d6756aa 100644
--- 
a/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch
+++ 
b/tools/patman/test/0002-fdt-Correct-cast-for-sandbox-in-fdtdec_setup_mem_siz.patch
@@ -21,6 +21,7 @@ Series-cc: Stefan Brüns 
 Cover-letter-cc: Lord Mëlchett 
 Series-version: 3
 Patch-cc: fred
+Commit-cc: joe
 Series-process-log: sort, uniq
 Series-changes: 4
 - Some changes
diff --git a/tools/patman/test/test01.txt b/tools/patman/test/test01.txt
index fc3066e50b4..271d9bf043f 100644
--- a/tools/patman/test/test01.txt
+++ b/tools/patman/test/test01.txt
@@ -49,6 +49,7 @@ Date:   Sat Apr 15 15:39:08 2017 -0600
 Cover-letter-cc: Lord Mëlchett 
 Series-version: 3
 Patch-cc: fred
+Commit-cc: joe
 Series-process-log: sort, uniq
 Series-changes: 4
 - Some changes
-- 
2.37.1

[PATCH 1/3] patman: Fix tests if add_maintainers is set to False

[Sean Anderson]

If add_maintainers is set to False in the user's ~/.patman config, it will
cause the custom_get_maintainer_script to fail since that test expects
maintainers to be added. Set add_maintainer to True in the .patman config
to prevent this.

Fixes: 8c042fb7f9f ("patman: add '--get-maintainer-script' argument")
Signed-off-by: Sean Anderson 
---

 tools/patman/func_test.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index e3918497cf4..9c016fb5e9a 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -540,7 +540,8 @@ complicated as possible''')
 with open('.patman', 'w', buffering=1) as f:
 f.write('[settings]\n'
 'get_maintainer_script: dummy-script.sh\n'
-'check_patch: False\n')
+'check_patch: False\n'
+'add_maintainers: True\n')
 with open('dummy-script.sh', 'w', buffering=1) as f:
 f.write('#!/usr/bin/env python\n'
 'print("he...@there.com")\n')
-- 
2.37.1

[PATCH 0/3] patman: A fix and some new tags

[Sean Anderson]

This series has a fix along with a couple of convenient tags.


Sean Anderson (3):
  patman: Fix tests if add_maintainers is set to False
  patman: Add Commit-cc as an alias for Patch-cc
  patman: Add a tag for when a patch gets added to a series

 tools/patman/func_test.py | 10 --
 tools/patman/patchstream.py   |  7 +++
 tools/patman/patman.rst   | 15 ++-
 ...cast-for-sandbox-in-fdtdec_setup_mem_siz.patch |  2 ++
 tools/patman/test/test01.txt  |  2 ++
 5 files changed, 33 insertions(+), 3 deletions(-)

-- 
2.37.1

Re: [PATCH] mmc: sdhci: programmable clock calculation needs multiplier +1

[Sean Anderson]

On 4/12/24 15:26, curtis.mach...@intel.com wrote:
> [You don't often get email from curtis.mach...@intel.com. Learn why this is 
> important at 
> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2faka.ms%2fLearnAboutSenderIdentification&umid=4f25273f-b33b-4eb3-8643-c1f6eff8842b&auth=d807158c60b7d2502abde8a2fc01f40662980862-c95c2e418b830130ad99c91e42b2732e3f7e44ee
>  ]
> 
> From: cmachida 
> 
> According to the SD Host Controller Simplified Specification v4.20,
> the multiplier value M is one more than the Clock Multiplier field.
> 
> Copied code from Linux project.  drivers/mmc/host/sdhci.c line 4405
> 
> Signed-off-by: cmachida 
> ---
> 
>  drivers/mmc/sdhci.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 0178ed8a11..a8476ec4e9 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -929,6 +929,15 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct 
> sdhci_host *host,
> debug("%s, caps_1: 0x%x\n", __func__, caps_1);
> host->clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >>
> SDHCI_CLOCK_MUL_SHIFT;
> +
> +   /*
> +* In case the value in Clock Multiplier is 0, then 
> programmable
> +* clock mode is not supported, otherwise the actual clock
> +* multiplier is one more than the value of Clock Multiplier
> +* in the Capabilities Register.
> +*/
> +   if (host->clk_mul)
> +   host->clk_mul += 1;
> }
> 
> if (host->max_clk == 0) {
> --
> 2.43.2
> 

Reviewed-by: Sean Anderson 

Re: [PATCH 1/1] clk: sifive: append missing \n to messages

[Sean Anderson]

On 4/11/24 04:37, Heinrich Schuchardt wrote:

On 11.04.24 05:13, Sean Anderson wrote:

On 2/16/24 11:35, Heinrich Schuchardt wrote:

If multiple messages are written, line-feeds improve the readability.

Fixes: c40b6df87fc0 ("clk: Add SiFive FU540 PRCI clock driver")
Signed-off-by: Heinrich Schuchardt 
---
  drivers/clk/analogbits/wrpll-cln28hpc.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/analogbits/wrpll-cln28hpc.c 
b/drivers/clk/analogbits/wrpll-cln28hpc.c
index a3cb109d357..537c696b727 100644
--- a/drivers/clk/analogbits/wrpll-cln28hpc.c
+++ b/drivers/clk/analogbits/wrpll-cln28hpc.c
@@ -81,7 +81,7 @@ static int __wrpll_calc_filter_range(unsigned long 
post_divr_freq)
  {
  if (post_divr_freq < MIN_POST_DIVR_FREQ ||
  post_divr_freq > MAX_POST_DIVR_FREQ) {
-    WARN(1, "%s: post-divider reference freq out of range: %lu",
+    WARN(1, "%s: post-divider reference freq out of range: %lu\n",
   __func__, post_divr_freq);
  return -ERANGE;
  }
@@ -229,7 +229,7 @@ int wrpll_configure_for_rate(struct wrpll_cfg *c, u32 
target_rate,
  int range;
  if (c->flags == 0) {
-    WARN(1, "%s called with uninitialized PLL config", __func__);
+    WARN(1, "%s called with uninitialized PLL config\n", __func__);
  return -EINVAL;
  }
@@ -335,7 +335,7 @@ unsigned long wrpll_calc_output_rate(const struct wrpll_cfg 
*c,
  u64 n;
  if (c->flags & WRPLL_FLAGS_EXT_FEEDBACK_MASK) {
-    WARN(1, "external feedback mode not yet supported");
+    WARN(1, "external feedback mode not yet supported\n");
  return ULONG_MAX;
  }


Reviewed-by: Sean Anderson 

But maybe these should be dev_dbg?


The messages look like indicating misconfiguration. So the user should see 
these.


Yeah, but we already return an error. So the user will notice.


The kernel code also uses WARN() (and also lacks the line-feeds).


In the kernel there are no size restrictions, so it is fine to include messages.

--Sean

Re: [PATCH] clk: imx8mp: add pwm clocks support

[Sean Anderson]

On 4/10/24 23:18, Sean Anderson wrote:

On 3/10/23 10:15, Tommaso Merciai wrote:

Add clocks support for the PWM controllers. This is ported from
Linux v6.3.0-rc1

Signed-off-by: Tommaso Merciai 
---
  drivers/clk/imx/clk-imx8mp.c | 24 
  1 file changed, 24 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index ffbc1d1ba9..fac87ff505 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -122,6 +122,22 @@ static const char *imx8mp_gic_sels[] = {"clock-osc-24m", 
"sys_pll2_200m", "sys_p
  "sys_pll2_100m", "sys_pll1_800m",
  "sys_pll2_500m", "clk_ext4", "audio_pll2_out" };
+static const char * const imx8mp_pwm1_sels[] = {"clock-osc-24m", "sys_pll2_100m", 
"sys_pll1_160m",
+    "sys_pll1_40m", "sys_pll3_out", "clk_ext1",
+    "sys_pll1_80m", "video_pll1_out", };
+
+static const char * const imx8mp_pwm2_sels[] = {"clock-osc-24m", "sys_pll2_100m", 
"sys_pll1_160m",
+    "sys_pll1_40m", "sys_pll3_out", "clk_ext1",
+    "sys_pll1_80m", "video_pll1_out", };
+
+static const char * const imx8mp_pwm3_sels[] = {"clock-osc-24m", "sys_pll2_100m", 
"sys_pll1_160m",
+    "sys_pll1_40m", "sys_pll3_out", "clk_ext2",
+    "sys_pll1_80m", "video_pll1_out", };
+
+static const char * const imx8mp_pwm4_sels[] = {"clock-osc-24m", "sys_pll2_100m", 
"sys_pll1_160m",
+    "sys_pll1_40m", "sys_pll3_out", "clk_ext2",
+    "sys_pll1_80m", "video_pll1_out", };
+
  static const char *imx8mp_ecspi1_sels[] = {"clock-osc-24m", "sys_pll2_200m", 
"sys_pll1_40m",
    "sys_pll1_160m", "sys_pll1_800m", "sys_pll3_out",
    "sys_pll2_250m", "audio_pll2_out", };
@@ -270,6 +286,10 @@ static int imx8mp_clk_probe(struct udevice *dev)
  clk_dm(IMX8MP_CLK_GIC, imx8m_clk_composite_critical("gic", 
imx8mp_gic_sels, base + 0xb200));
  clk_dm(IMX8MP_CLK_ECSPI1, imx8m_clk_composite("ecspi1", 
imx8mp_ecspi1_sels, base + 0xb280));
  clk_dm(IMX8MP_CLK_ECSPI2, imx8m_clk_composite("ecspi2", 
imx8mp_ecspi2_sels, base + 0xb300));
+    clk_dm(IMX8MP_CLK_PWM1, imx8m_clk_composite_critical("pwm1", 
imx8mp_pwm1_sels, base + 0xb380));
+    clk_dm(IMX8MP_CLK_PWM2, imx8m_clk_composite_critical("pwm2", 
imx8mp_pwm2_sels, base + 0xb400));
+    clk_dm(IMX8MP_CLK_PWM3, imx8m_clk_composite_critical("pwm3", 
imx8mp_pwm3_sels, base + 0xb480));
+    clk_dm(IMX8MP_CLK_PWM4, imx8m_clk_composite_critical("pwm4", 
imx8mp_pwm4_sels, base + 0xb500));
  clk_dm(IMX8MP_CLK_ECSPI3, imx8m_clk_composite("ecspi3", 
imx8mp_ecspi3_sels, base + 0xc180));
  clk_dm(IMX8MP_CLK_WDOG, imx8m_clk_composite("wdog", imx8mp_wdog_sels, 
base + 0xb900));
@@ -292,6 +312,10 @@ static int imx8mp_clk_probe(struct udevice *dev)
  clk_dm(IMX8MP_CLK_I2C2_ROOT, imx_clk_gate4("i2c2_root_clk", "i2c2", base 
+ 0x4180, 0));
  clk_dm(IMX8MP_CLK_I2C3_ROOT, imx_clk_gate4("i2c3_root_clk", "i2c3", base 
+ 0x4190, 0));
  clk_dm(IMX8MP_CLK_I2C4_ROOT, imx_clk_gate4("i2c4_root_clk", "i2c4", base 
+ 0x41a0, 0));
+    clk_dm(IMX8MP_CLK_PWM1_ROOT, imx_clk_gate4("pwm1_root_clk", "pwm1", base + 
0x4280, 0));
+    clk_dm(IMX8MP_CLK_PWM2_ROOT, imx_clk_gate4("pwm2_root_clk", "pwm2", base + 
0x4290, 0));
+    clk_dm(IMX8MP_CLK_PWM3_ROOT, imx_clk_gate4("pwm3_root_clk", "pwm3", base + 
0x42a0, 0));
+    clk_dm(IMX8MP_CLK_PWM4_ROOT, imx_clk_gate4("pwm4_root_clk", "pwm4", base + 
0x42b0, 0));
  clk_dm(IMX8MP_CLK_QSPI_ROOT, imx_clk_gate4("qspi_root_clk", "qspi", base 
+ 0x42f0, 0));
  clk_dm(IMX8MP_CLK_I2C5_ROOT, imx_clk_gate2("i2c5_root_clk", "i2c5", base 
+ 0x4330, 0));
  clk_dm(IMX8MP_CLK_I2C6_ROOT, imx_clk_gate2("i2c6_root_clk", "i2c6", base 
+ 0x4340, 0));


Acked-by: Sean Anderson 

But I would like to see a RB from one of the i.MX maintainers (CC'd).


Whoops, didn't notice the date. Seems like this has been added in

https://lore.kernel.org/u-boot/20230330152333.53b1185...@phobos.denx.de/

Re: [PATCH] clk: imx8mp: add pwm clocks support

[Sean Anderson]

On 3/10/23 10:15, Tommaso Merciai wrote:

Add clocks support for the PWM controllers. This is ported from
Linux v6.3.0-rc1

Signed-off-by: Tommaso Merciai 
---
  drivers/clk/imx/clk-imx8mp.c | 24 
  1 file changed, 24 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index ffbc1d1ba9..fac87ff505 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -122,6 +122,22 @@ static const char *imx8mp_gic_sels[] = {"clock-osc-24m", 
"sys_pll2_200m", "sys_p
"sys_pll2_100m", "sys_pll1_800m",
"sys_pll2_500m", "clk_ext4", 
"audio_pll2_out" };
  
+static const char * const imx8mp_pwm1_sels[] = {"clock-osc-24m", "sys_pll2_100m", "sys_pll1_160m",

+   "sys_pll1_40m", "sys_pll3_out", 
"clk_ext1",
+   "sys_pll1_80m", "video_pll1_out", };
+
+static const char * const imx8mp_pwm2_sels[] = {"clock-osc-24m", "sys_pll2_100m", 
"sys_pll1_160m",
+   "sys_pll1_40m", "sys_pll3_out", 
"clk_ext1",
+   "sys_pll1_80m", "video_pll1_out", };
+
+static const char * const imx8mp_pwm3_sels[] = {"clock-osc-24m", "sys_pll2_100m", 
"sys_pll1_160m",
+   "sys_pll1_40m", "sys_pll3_out", 
"clk_ext2",
+   "sys_pll1_80m", "video_pll1_out", };
+
+static const char * const imx8mp_pwm4_sels[] = {"clock-osc-24m", "sys_pll2_100m", 
"sys_pll1_160m",
+   "sys_pll1_40m", "sys_pll3_out", 
"clk_ext2",
+   "sys_pll1_80m", "video_pll1_out", };
+
  static const char *imx8mp_ecspi1_sels[] = {"clock-osc-24m", "sys_pll2_200m", 
"sys_pll1_40m",
  "sys_pll1_160m", "sys_pll1_800m", 
"sys_pll3_out",
  "sys_pll2_250m", 
"audio_pll2_out", };
@@ -270,6 +286,10 @@ static int imx8mp_clk_probe(struct udevice *dev)
clk_dm(IMX8MP_CLK_GIC, imx8m_clk_composite_critical("gic", 
imx8mp_gic_sels, base + 0xb200));
clk_dm(IMX8MP_CLK_ECSPI1, imx8m_clk_composite("ecspi1", 
imx8mp_ecspi1_sels, base + 0xb280));
clk_dm(IMX8MP_CLK_ECSPI2, imx8m_clk_composite("ecspi2", 
imx8mp_ecspi2_sels, base + 0xb300));
+   clk_dm(IMX8MP_CLK_PWM1, imx8m_clk_composite_critical("pwm1", 
imx8mp_pwm1_sels, base + 0xb380));
+   clk_dm(IMX8MP_CLK_PWM2, imx8m_clk_composite_critical("pwm2", 
imx8mp_pwm2_sels, base + 0xb400));
+   clk_dm(IMX8MP_CLK_PWM3, imx8m_clk_composite_critical("pwm3", 
imx8mp_pwm3_sels, base + 0xb480));
+   clk_dm(IMX8MP_CLK_PWM4, imx8m_clk_composite_critical("pwm4", 
imx8mp_pwm4_sels, base + 0xb500));
clk_dm(IMX8MP_CLK_ECSPI3, imx8m_clk_composite("ecspi3", 
imx8mp_ecspi3_sels, base + 0xc180));
  
  	clk_dm(IMX8MP_CLK_WDOG, imx8m_clk_composite("wdog", imx8mp_wdog_sels, base + 0xb900));

@@ -292,6 +312,10 @@ static int imx8mp_clk_probe(struct udevice *dev)
clk_dm(IMX8MP_CLK_I2C2_ROOT, imx_clk_gate4("i2c2_root_clk", "i2c2", 
base + 0x4180, 0));
clk_dm(IMX8MP_CLK_I2C3_ROOT, imx_clk_gate4("i2c3_root_clk", "i2c3", 
base + 0x4190, 0));
clk_dm(IMX8MP_CLK_I2C4_ROOT, imx_clk_gate4("i2c4_root_clk", "i2c4", 
base + 0x41a0, 0));
+   clk_dm(IMX8MP_CLK_PWM1_ROOT, imx_clk_gate4("pwm1_root_clk", "pwm1", 
base + 0x4280, 0));
+   clk_dm(IMX8MP_CLK_PWM2_ROOT, imx_clk_gate4("pwm2_root_clk", "pwm2", 
base + 0x4290, 0));
+   clk_dm(IMX8MP_CLK_PWM3_ROOT, imx_clk_gate4("pwm3_root_clk", "pwm3", 
base + 0x42a0, 0));
+   clk_dm(IMX8MP_CLK_PWM4_ROOT, imx_clk_gate4("pwm4_root_clk", "pwm4", 
base + 0x42b0, 0));
clk_dm(IMX8MP_CLK_QSPI_ROOT, imx_clk_gate4("qspi_root_clk", "qspi", 
base + 0x42f0, 0));
clk_dm(IMX8MP_CLK_I2C5_ROOT, imx_clk_gate2("i2c5_root_clk", "i2c5", 
base + 0x4330, 0));
clk_dm(IMX8MP_CLK_I2C6_ROOT, imx_clk_gate2("i2c6_root_clk", "i2c6", 
base + 0x4340, 0));


Acked-by: Sean Anderson 

But I would like to see a RB from one of the i.MX maintainers (CC'd).

Re: [PATCH 1/1] clk: sifive: avoid declaring static variables in includes

[Sean Anderson]

On 2/16/24 18:18, Heinrich Schuchardt wrote:

The existing code is unnecessarily convoluted:

Arrays __prci_init_clocks_fu[5|7]40  are initialized with data.
In separate includes fu[5|7]40-prci.h the size of the arrays is provided as
constants.

By moving the structures prci_clk_fu[5|7]40 to the respective code modules
we can directly use ARRAY_SIZE() to access the size of the data used for
initialization.

Signed-off-by: Heinrich Schuchardt 
---
  drivers/clk/sifive/fu540-prci.c  |  7 ++-
  drivers/clk/sifive/fu540-prci.h  | 22 --
  drivers/clk/sifive/fu740-prci.c  |  7 ++-
  drivers/clk/sifive/fu740-prci.h  | 22 --
  drivers/clk/sifive/sifive-prci.c |  3 +--
  drivers/clk/sifive/sifive-prci.h |  4 
  6 files changed, 17 insertions(+), 48 deletions(-)
  delete mode 100644 drivers/clk/sifive/fu540-prci.h
  delete mode 100644 drivers/clk/sifive/fu740-prci.h

diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c
index ceb2c6fab0d..e55a26ab8fd 100644
--- a/drivers/clk/sifive/fu540-prci.c
+++ b/drivers/clk/sifive/fu540-prci.c
@@ -58,7 +58,7 @@ static const struct __prci_clock_ops 
sifive_fu540_prci_tlclksel_clk_ops = {
  };
  
  /* List of clock controls provided by the PRCI */

-struct __prci_clock __prci_init_clocks_fu540[] = {
+static struct __prci_clock __prci_init_clocks_fu540[] = {
[PRCI_CLK_COREPLL] = {
.name = "corepll",
.parent_name = "hfclk",
@@ -83,3 +83,8 @@ struct __prci_clock __prci_init_clocks_fu540[] = {
.ops = &sifive_fu540_prci_tlclksel_clk_ops,
},
  };
+
+const struct prci_clk_desc prci_clk_fu540 = {
+   .clks = __prci_init_clocks_fu540,
+   .num_clks = ARRAY_SIZE(__prci_init_clocks_fu540),
+};
diff --git a/drivers/clk/sifive/fu540-prci.h b/drivers/clk/sifive/fu540-prci.h
deleted file mode 100644
index 113301107da..000
--- a/drivers/clk/sifive/fu540-prci.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright (C) 2020-2021 SiFive, Inc.
- * Zong Li
- * Pragnesh Patel
- */
-
-#ifndef __SIFIVE_CLK_FU540_PRCI_H
-#define __SIFIVE_CLK_FU540_PRCI_H
-
-#include "sifive-prci.h"
-
-#define NUM_CLOCK_FU5404
-
-extern struct __prci_clock __prci_init_clocks_fu540[NUM_CLOCK_FU540];
-
-static const struct prci_clk_desc prci_clk_fu540 = {
-   .clks = __prci_init_clocks_fu540,
-   .num_clks = ARRAY_SIZE(__prci_init_clocks_fu540),
-};
-
-#endif /* __SIFIVE_CLK_FU540_PRCI_H */
diff --git a/drivers/clk/sifive/fu740-prci.c b/drivers/clk/sifive/fu740-prci.c
index 5edc864e4bd..4274b215d2f 100644
--- a/drivers/clk/sifive/fu740-prci.c
+++ b/drivers/clk/sifive/fu740-prci.c
@@ -102,7 +102,7 @@ static const struct __prci_clock_ops 
sifive_fu740_prci_pcieaux_clk_ops = {
  };
  
  /* List of clock controls provided by the PRCI */

-struct __prci_clock __prci_init_clocks_fu740[] = {
+static struct __prci_clock __prci_init_clocks_fu740[] = {
[FU740_PRCI_CLK_COREPLL] = {
.name = "corepll",
.parent_name = "hfclk",
@@ -156,3 +156,8 @@ struct __prci_clock __prci_init_clocks_fu740[] = {
.pwd = &__prci_pcieaux_data,
}
  };
+
+const struct prci_clk_desc prci_clk_fu740 = {
+   .clks = __prci_init_clocks_fu740,
+   .num_clks = ARRAY_SIZE(__prci_init_clocks_fu740),
+};
diff --git a/drivers/clk/sifive/fu740-prci.h b/drivers/clk/sifive/fu740-prci.h
deleted file mode 100644
index b74f0789061..000
--- a/drivers/clk/sifive/fu740-prci.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright (C) 2020-2021 SiFive, Inc.
- * Zong Li
- * Pragnesh Patel
- */
-
-#ifndef __SIFIVE_CLK_FU740_PRCI_H
-#define __SIFIVE_CLK_FU740_PRCI_H
-
-#include "sifive-prci.h"
-
-#define NUM_CLOCK_FU7409
-
-extern struct __prci_clock __prci_init_clocks_fu740[NUM_CLOCK_FU740];
-
-static const struct prci_clk_desc prci_clk_fu740 = {
-   .clks = __prci_init_clocks_fu740,
-   .num_clks = ARRAY_SIZE(__prci_init_clocks_fu740),
-};
-
-#endif /* __SIFIVE_CLK_FU740_PRCI_H */
diff --git a/drivers/clk/sifive/sifive-prci.c b/drivers/clk/sifive/sifive-prci.c
index c8fb6002907..603a176d06a 100644
--- a/drivers/clk/sifive/sifive-prci.c
+++ b/drivers/clk/sifive/sifive-prci.c
@@ -34,8 +34,7 @@
  #include 
  #include 
  
-#include "fu540-prci.h"

-#include "fu740-prci.h"
+#include "sifive-prci.h"
  
  /*

   * Private functions
diff --git a/drivers/clk/sifive/sifive-prci.h b/drivers/clk/sifive/sifive-prci.h
index 5ce33d61846..b391698081d 100644
--- a/drivers/clk/sifive/sifive-prci.h
+++ b/drivers/clk/sifive/sifive-prci.h
@@ -320,4 +320,8 @@ unsigned long sifive_prci_hfpclkplldiv_recalc_rate(struct 
__prci_clock *pc,
  
  int sifive_prci_clock_enable(struct __prci_clock *pc, bool enable);
  
+/* Clock driver data */

+extern const struct prci_clk_desc prci_clk_fu540;
+extern const struct prci_clk_desc prci_clk_fu740;
+
  #endif /* __SIFIVE_CLK

Re: [PATCH] clk: sifive: check wrpll_configure_for_rate() return value

[Sean Anderson]

On 2/16/24 12:06, Heinrich Schuchardt wrote:

wrpll_configure_for_rate() might fail. We should check the return value.

Fixes: d56d79ed27c6 ("drivers: clk: add fu740 support")
Signed-off-by: Heinrich Schuchardt 
---
  drivers/clk/sifive/sifive-prci.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/sifive/sifive-prci.c b/drivers/clk/sifive/sifive-prci.c
index c8fb6002907..a950736f11b 100644
--- a/drivers/clk/sifive/sifive-prci.c
+++ b/drivers/clk/sifive/sifive-prci.c
@@ -209,7 +209,9 @@ unsigned long sifive_prci_wrpll_round_rate(struct 
__prci_clock *pc,
  
  	memcpy(&c, &pwd->c, sizeof(c));
  
-	wrpll_configure_for_rate(&c, rate, *parent_rate);

+   r = wrpll_configure_for_rate(&c, rate, *parent_rate);
+   if (r)
+   return r;
  
  	return wrpll_calc_output_rate(&c, *parent_rate);

  }


Reviewed-by: Sean Anderson 

Re: [PATCH 1/1] clk: sifive: append missing \n to messages

[Sean Anderson]

On 2/16/24 11:35, Heinrich Schuchardt wrote:

If multiple messages are written, line-feeds improve the readability.

Fixes: c40b6df87fc0 ("clk: Add SiFive FU540 PRCI clock driver")
Signed-off-by: Heinrich Schuchardt 
---
  drivers/clk/analogbits/wrpll-cln28hpc.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/analogbits/wrpll-cln28hpc.c 
b/drivers/clk/analogbits/wrpll-cln28hpc.c
index a3cb109d357..537c696b727 100644
--- a/drivers/clk/analogbits/wrpll-cln28hpc.c
+++ b/drivers/clk/analogbits/wrpll-cln28hpc.c
@@ -81,7 +81,7 @@ static int __wrpll_calc_filter_range(unsigned long 
post_divr_freq)
  {
if (post_divr_freq < MIN_POST_DIVR_FREQ ||
post_divr_freq > MAX_POST_DIVR_FREQ) {
-   WARN(1, "%s: post-divider reference freq out of range: %lu",
+   WARN(1, "%s: post-divider reference freq out of range: %lu\n",
 __func__, post_divr_freq);
return -ERANGE;
}
@@ -229,7 +229,7 @@ int wrpll_configure_for_rate(struct wrpll_cfg *c, u32 
target_rate,
int range;
  
  	if (c->flags == 0) {

-   WARN(1, "%s called with uninitialized PLL config", __func__);
+   WARN(1, "%s called with uninitialized PLL config\n", __func__);
return -EINVAL;
}
  
@@ -335,7 +335,7 @@ unsigned long wrpll_calc_output_rate(const struct wrpll_cfg *c,

u64 n;
  
  	if (c->flags & WRPLL_FLAGS_EXT_FEEDBACK_MASK) {

-   WARN(1, "external feedback mode not yet supported");
+   WARN(1, "external feedback mode not yet supported\n");
return ULONG_MAX;
}
  


Reviewed-by: Sean Anderson 

But maybe these should be dev_dbg?

Re: [PATCH] clk: Fix error message in clk_get_bulk

[Sean Anderson]

On 3/9/24 07:27, Jan Kiszka wrote:

From: Jan Kiszka 

Fix a logical inversion of the printed text.

Signed-off-by: Jan Kiszka 
---
  drivers/clk/clk-uclass.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index ed6e60bc484..78d8ea94c65 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -180,7 +180,7 @@ int clk_get_bulk(struct udevice *dev, struct clk_bulk *bulk)
  bulk_get_err:
err = clk_release_all(bulk->clks, bulk->count);
if (err)
-   debug("%s: could release all clocks for %p\n",
+   debug("%s: could not release all clocks for %p\n",
  __func__, dev);
  
  	return ret;


Reviewed-by: Sean Anderson 

Re: [PATCH v3] clk: set initial best mux parent to current parent with CLK_MUX_ROUND_CLOSEST

[Sean Anderson]

On 3/12/24 04:52, Yang Xiwen wrote:

On 3/11/2024 5:34 PM, Maxime Ripard wrote:

On Thu, Mar 07, 2024 at 07:18:05PM +0800, Yang Xiwen wrote:

On 3/7/2024 4:48 PM, Maxime Ripard wrote:

Hi,

On Thu, Mar 07, 2024 at 10:03:50AM +0800, Yang Xiwen via B4 Relay wrote:

From: Yang Xiwen 

Originally, the initial clock rate is hardcoded to 0, this can lead to
some problem when setting a very small rate with CLK_MUX_ROUND_CLOSEST.

For example, if the lowest possible rate provided by the mux is 1000Hz,
setting a rate below 500Hz will fail, because no clock can provide a
better rate than the non-existant 0Hz. But it should succeed with 1000Hz
being set.

Setting the initial best parent to current parent could solve this bug.

Signed-off-by: Yang Xiwen 

I don't think it would be the way to go. The biggest issue to me is that
it's inconsistent, and only changing the behaviour for a given flag
doesn't solve that.


I think the current behavior is odd but conforms to the document if
CLK_MUX_ROUND_CLOSEST is not specified.

clk_mux_determine_rate_flags isn't documented, and the determine_rate
clk_ops documentation doesn't mention it can return an error.


If i understand correctly, the default behavior of mux clocks is to
select the closest rate lower than requested rate, and
CLK_MUX_ROUND_CLOSEST removes the "lower than" limitation, which is
what this version tries to accomplish.

The situation is not as clear-cut as you make it to be, unfortunately.
The determine_rate clk_ops implementation states:

   Given a target rate as input, returns the closest rate actually
   supported by the clock, and optionally the parent clock that should be
   used to provide the clock rate.

So CLK_MUX_ROUND_CLOSEST shouldn't exist, because that's what
determine_rate expects so it should always be there.

Now, the "actually supported by the clock" can be interpreted in
multiple ways, and most importantly, doesn't state what the behaviour is
if we can't find a rate actually supported by the clock.

But now, this situation has been ambiguous for a while and thus drivers
kind of relied on that ambiguity.

So the way to fix it up is:

   - Assess what drivers are relying on
   - Document the current behaviour in clk_ops determine_rate



 From my investigation, it's totally a mess, especially for platform clk 
drivers (PLL). Some drivers always round down, the others round to nearest, 
with or without a specific flag to switch between them, depend on the division 
functions they choose. Fixing all of them seems needs quite a lot of time and 
would probably introduce some regressions.

We'd probably only have to say both rounding to nearest and rounding down are 
acceptable, though either one is preferred.




   - Make clk_mux_determine_rate_flags consistent with that



I think we must keep existing flags and document the current behavior correctly 
because of the massive existing users of clk_mux.


That's why i'm going to only fix CLK_MUX_ROUND_CLOSEST users. Hopefully it 
won't cause too many regressions.



   - Run that through kernelci to make sure we don't have any regression



We don't. I run 'tools/testing/kunit/kunit.py run --kunitconfig 
drivers/clk/.kunitconfig' each time before i send patches.


Over all, it seems quite a lot of work here.




Maxime



The situation here becomes even more complex when it comes to U-Boot clk framework. They chose slightly different prototypes and stated clk_set_rate() can fail with -ve. 


Maybe you mean clk_get_rate? Setting a rate can always fail due to the
nature of clocks...

Personally, I am not terribly attached to the API (as not many callers
handle errors correctly), but I have not had the time recently to do any
cleanup.

It's a great burden for clk driver authors and maintainers when they try to 
port their drivers to U-Boot. Let's Cc U-Boot clk maintainers as well, and see 
how we can resolve the mess here.




Regarding rounding mode, IMO it is better to let driver authors pick
whatever is convenient. Round closest is best, but there may be code size
savings for round lowest or some other scheme. [1] has the current
recommendation, which is to punt and let the caller use round_rate if
they actually care.

--Sean

[1] 
https://source.denx.de/u-boot/custodians/u-boot-clk/-/blob/a8dc4965f09d28a59c156437673ddb66860c847e/include/clk-uclass.h#L143

Re: [PATCH] clk: Propagate clk_set_rate() if CLK_SET_PARENT_RATE present

[Sean Anderson]

On 3/7/24 19:04, Sam Protsenko wrote:

Sometimes clocks provided to a consumer might not have .set_rate
operation (like gate or mux clocks), but have CLK_SET_PARENT_RATE flag
set. In that case it's usually possible to find a parent up the tree
which is capable of setting the rate (div, pll, etc). Implement a simple
lookup procedure for such cases, to traverse the clock tree until
.set_rate capable parent is found, and use that parent to actually
change the rate. The search will stop once the first .set_rate capable
clock is found, which is usually enough to handle most cases.

Signed-off-by: Sam Protsenko 
---
  drivers/clk/clk-uclass.c | 16 ++--
  1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index ed6e60bc4841..755f05f34669 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -571,8 +571,20 @@ ulong clk_set_rate(struct clk *clk, ulong rate)
return 0;
ops = clk_dev_ops(clk->dev);
  
-	if (!ops->set_rate)

-   return -ENOSYS;
+   /* Try to find parents which can set rate */
+   while (!ops->set_rate) {
+   struct clk *parent;
+
+   if (!(clk->flags & CLK_SET_RATE_PARENT))
+   return -ENOSYS;
+
+   parent = clk_get_parent(clk);
+   if (IS_ERR_OR_NULL(parent) || !clk_valid(parent))
+   return -ENODEV;
+
+   clk = parent;
+   ops = clk_dev_ops(clk->dev);
+   }
  
  	/* get private clock struct used for cache */

clk_get_priv(clk, &clkp);


Can you give an example of where this is needed?

--Sean

Re: [PATCH 1/1] sandbox: use sane access rights for files

[Sean Anderson]

On 4/10/24 04:38, Heinrich Schuchardt wrote:

When writing an executable, allowing other users to modify it introduces
a security issue.

Generally we should avoid giving other users write access to our files by
default.

Replace chmod(777) by chmod(755) and chmod(644).

Fixes: 47f5fcfb4169 ("sandbox: Add os_jump_to_image() to run another 
executable")
Fixes: d9165153caea ("sandbox: add flags for open() call")
Fixes: 5c2859cdc302 ("sandbox: Allow reading/writing of RAM buffer")
Signed-off-by: Heinrich Schuchardt 
---
  arch/sandbox/cpu/os.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index cbae5109e85..1cf41578010 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -109,7 +109,7 @@ int os_open(const char *pathname, int os_flags)
 */
flags |= O_CLOEXEC;
  
-	return open(pathname, flags, 0777);

+   return open(pathname, flags, 0644);
  }
  
  int os_close(int fd)

@@ -746,7 +746,7 @@ int os_write_ram_buf(const char *fname)
struct sandbox_state *state = state_get_current();
int fd, ret;
  
-	fd = open(fname, O_CREAT | O_WRONLY, 0777);

+   fd = open(fname, O_CREAT | O_WRONLY, 0644);
if (fd < 0)
return -ENOENT;
ret = write(fd, state->ram_buf, state->ram_size);
@@ -791,7 +791,7 @@ static int make_exec(char *fname, const void *data, int 
size)
if (write(fd, data, size) < 0)
return -EIO;
close(fd);
-   if (chmod(fname, 0777))
+   if (chmod(fname, 0755))
return -ENOEXEC;
  
  	return 0;


Reviewed-by: Sean Anderson 

Re: [PATCH 1/1] sandbox: don't call os_close with invalid file descriptor

[Sean Anderson]

On 4/10/24 17:50, Heinrich Schuchardt wrote:

If open() fails it returns -1. Calling close() with this value
makes no sense. Return -EIO instead.

Addresses-Coverity-ID: 185828 Improper use of negative value
Signed-off-by: Heinrich Schuchardt 
---
  arch/sandbox/cpu/os.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index cbae5109e85..154a5d77490 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -188,7 +188,7 @@ int os_read_file(const char *fname, void **bufp, int *sizep)
fd = os_open(fname, OS_O_RDONLY);
if (fd < 0) {
printf("Cannot open file '%s'\n", fname);
-   goto err;
+   return -EIO;
}
size = os_filesize(fd);
if (size < 0) {


Fixes: 566bf3a8698 ("sandbox: Add a function to read a host file")
Reviewed-by: Sean Anderson 

Re: [PATCH 1/2] dm: core: add support for fallback drivers

[Sean Anderson]

On 4/10/24 15:44, Tom Rini wrote:

On Wed, Apr 10, 2024 at 07:27:17PM +0200, Heinrich Schuchardt wrote:



Am 10. April 2024 19:06:57 MESZ schrieb Caleb Connolly 
:

Introduce support for a uclass to provide a fallback/stub driver which
can be used when no device is found for a given node. This might be
useful for handling non-essential clock controllers like the RPMh on
Qualcomm platforms, or during early bringup to get UART output before a
real clock driver has been created.

Signed-off-by: Caleb Connolly 
---
drivers/core/Kconfig  | 10 ++
drivers/core/uclass.c | 24 +++-
include/dm/uclass.h   |  3 +++
3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 1081d61fcf01..09075b9b7a15 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -466,5 +466,15 @@ config BOUNCE_BUFFER

  A second possible use of bounce buffers is their ability to
  provide aligned buffers for DMA operations.

+menuconfig FALLBACK_DRIVERS


Wouldn't it be preferable to mark individual drivers as fallback drivers in 
their declaration?

This would allow alternative fallback drivers and would not require any 
definitions at uclass level.

Just by building a fallback driver you would enable the fallback behavior for 
its uclass.


I think some of this is addressed in the cover letter. My concern /
questions come down to I think just a matter of naming. Both "fallback"
and "stub" are used at times. As a concept, we have some areas where we
need a no-op driver because whereas the DT describes a relationship in
the hardware, here in U-Boot we can just accept things as configured. To
me "fallback" implies more functionality of the driver when it's really
just a generic no-op driver to fill in the dependencies within the tree.
Can we rename things a bit along those lines?



I would rather just have a stub driver with compatibles for all clocks we want
it to match. I think having a generic fallback driver could cause issues in the
future if we need to switch over to using a real driver.

--Sean

Re: [RFC PATCH v1 1/1] mmc: snps_sdhci: Add sdhci driver support for TH1520 SoC

[Sean Anderson]

On 3/30/24 13:59, Maksim Kiselev wrote:
> Add support for DesignWare SDHCI host controller on Alibaba TH1520 SoC
>
> Signed-off-by: Maksim Kiselev 
> ---
>  drivers/mmc/Kconfig  |  12 +
>  drivers/mmc/Makefile |   1 +
>  drivers/mmc/snps_sdhci.c | 464 +++
>  3 files changed, 477 insertions(+)
>  create mode 100644 drivers/mmc/snps_sdhci.c
>
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index cef05790dd..6936fa0928 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -671,6 +671,18 @@ config MMC_SDHCI_S5P
>
> If unsure, say N.
>
> +config MMC_SDHCI_SNPS
> + bool "Synopsys DesignWare SDHCI controller"
> + depends on MMC_SDHCI
> + depends on DM_MMC
> + help
> +   Support for DesignWare SDHCI host controller on Alibaba TH1520 SoC.
> +   This is a highly configurable and programmable, high performance
> +   Mobile Storage Host Controller (MSHC) with AXI as the bus interface
> +   for data transfer.
> +
> +   If unsure, say N.
> +

Why not use MMC_DW?

--Sean


[Embedded World 2024, SECO 
SpA]

Re: [PATCH] arm64: zynqmp: Also support JTAG as alternative boot mode

[Sean Anderson]

On 3/22/24 07:53, Michal Simek wrote:
> 
> 
> On 3/21/24 17:20, Sean Anderson wrote:
>> On 3/20/24 07:18, Michal Simek wrote:
>>> if (reg >> BOOT_MODE_ALT_SHIFT) condition rules out alternative jtag boot
>>> mode which is 0. When 0 was used origin(HW) boot mode was used instead.
>>> That's why directly fill reg variable with requested boot mode and don't
>>> let code to read value back. "else" part of code remain unchanged.
>>>
>>> Signed-off-by: Michal Simek 
>>> ---
>>>
>>>   arch/arm/mach-zynqmp/spl.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
>>> index 5af735aa5cef..979ff3aef6c2 100644
>>> --- a/arch/arm/mach-zynqmp/spl.c
>>> +++ b/arch/arm/mach-zynqmp/spl.c
>>> @@ -91,13 +91,14 @@ u32 spl_boot_device(void)
>>>     #if defined(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED)
>>>   /* Change default boot mode at run-time */
>>> +    reg = CONFIG_SPL_ZYNQMP_ALT_BOOTMODE;
>>>   writel(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE << BOOT_MODE_ALT_SHIFT,
>>>  &crlapb_base->boot_mode);
>>> -#endif
>>> -
>>> +#else
>>>   reg = readl(&crlapb_base->boot_mode);
>>>   if (reg >> BOOT_MODE_ALT_SHIFT)
>>>   reg >>= BOOT_MODE_ALT_SHIFT;
>>> +#endif
>>>     bootmode = reg & BOOT_MODES_MASK;
>>>   
>>
>> Looks fine; can we change this to
>>
>> if (IS_ENABLED(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED)) {
>> ...
>> } else {
>> ...
>> }
> 
> Issue is that CONFIG_SPL_ZYNQMP_ALT_BOOTMODE symbol is not defined and 
> depends on CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED. It means you get 
> compilation error.
> That symbol can be setup and then what you have above can work.
> Is it worth? TBH I don't have preference. Please take a look at patch below.
> (And if v1 is fine then at least there should be added depends on 
> SPL_ZYNQMP_ALT_BOOTMODE_ENABLED to SPL_ZYNQMP_ALT_BOOTMODE which is missing 
> there now).
> 
> Thanks,
> Michal
> 
> diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
> index eee34380f0a0..75d3ec916a66 100644
> --- a/arch/arm/mach-zynqmp/Kconfig
> +++ b/arch/arm/mach-zynqmp/Kconfig
> @@ -145,7 +145,7 @@ config ZYNQ_SDHCI_MAX_FREQ
> 
>  config SPL_ZYNQMP_ALT_BOOTMODE
>     hex
> -   default 0x0 if JTAG_MODE
> +   default 0x0
>     default 0x1 if QSPI_MODE_24BIT
>     default 0x2 if QSPI_MODE_32BIT
>     default 0x3 if SD_MODE
> diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
> index 979ff3aef6c2..bbbf684ae496 100644
> --- a/arch/arm/mach-zynqmp/spl.c
> +++ b/arch/arm/mach-zynqmp/spl.c
> @@ -89,16 +89,16 @@ u32 spl_boot_device(void)
>     u32 reg = 0;
>     u8 bootmode;
> 
> -#if defined(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED)
> -   /* Change default boot mode at run-time */
> -   reg = CONFIG_SPL_ZYNQMP_ALT_BOOTMODE;
> -   writel(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE << BOOT_MODE_ALT_SHIFT,
> -  &crlapb_base->boot_mode);
> -#else
> -   reg = readl(&crlapb_base->boot_mode);
> -   if (reg >> BOOT_MODE_ALT_SHIFT)
> -   reg >>= BOOT_MODE_ALT_SHIFT;
> -#endif
> +   if (IS_ENABLED(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED)) {
> +   /* Change default boot mode at run-time */
> +   reg = CONFIG_SPL_ZYNQMP_ALT_BOOTMODE;
> +   writel(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE << BOOT_MODE_ALT_SHIFT,
> +  &crlapb_base->boot_mode);
> +   } else {
> +   reg = readl(&crlapb_base->boot_mode);
> +   if (reg >> BOOT_MODE_ALT_SHIFT)
> +   reg >>= BOOT_MODE_ALT_SHIFT;
> +   }
> 
>     bootmode = reg & BOOT_MODES_MASK;
> 
> 
> 

Reviewed-by: Sean Anderson 

Re: [PATCH] arm64: zynqmp: Also support JTAG as alternative boot mode

[Sean Anderson]

On 3/20/24 07:18, Michal Simek wrote:
> if (reg >> BOOT_MODE_ALT_SHIFT) condition rules out alternative jtag boot
> mode which is 0. When 0 was used origin(HW) boot mode was used instead.
> That's why directly fill reg variable with requested boot mode and don't
> let code to read value back. "else" part of code remain unchanged.
> 
> Signed-off-by: Michal Simek 
> ---
> 
>  arch/arm/mach-zynqmp/spl.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
> index 5af735aa5cef..979ff3aef6c2 100644
> --- a/arch/arm/mach-zynqmp/spl.c
> +++ b/arch/arm/mach-zynqmp/spl.c
> @@ -91,13 +91,14 @@ u32 spl_boot_device(void)
>  
>  #if defined(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED)
>   /* Change default boot mode at run-time */
> + reg = CONFIG_SPL_ZYNQMP_ALT_BOOTMODE;
>   writel(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE << BOOT_MODE_ALT_SHIFT,
>  &crlapb_base->boot_mode);
> -#endif
> -
> +#else
>   reg = readl(&crlapb_base->boot_mode);
>   if (reg >> BOOT_MODE_ALT_SHIFT)
>   reg >>= BOOT_MODE_ALT_SHIFT;
> +#endif
>  
>   bootmode = reg & BOOT_MODES_MASK;
>  

Looks fine; can we change this to

if (IS_ENABLED(CONFIG_SPL_ZYNQMP_ALT_BOOTMODE_ENABLED)) {
...
} else {
...
}

?

--Sean

Re: [PATCH 1/2] clk: clk-imx8qxp: Add LPUART IPG entries

[Sean Anderson]

On 3/19/24 15:04, Tom Rini wrote:

On Mon, Mar 18, 2024 at 09:14:53PM -0300, Fabio Estevam wrote:

Hi Tom and Sean,

On Fri, Mar 8, 2024 at 5:13 PM Fabio Estevam  wrote:


Since commit cc7df0b9e8bc ("serial: lpuart: Enable IPG clock")
the colibri-imx8qxp board no longer boots.

The reason is that the imx8qxp clock driver does not handle the
LPUART IPG clocks inside get_rate(), set_rate() and enable() functions.

Fix the boot regression by adding the LPUART IPG entries.

Fixes: cc7df0b9e8bc ("serial: lpuart: Enable IPG clock")
Reported-by: Marcel Ziswiler 
Signed-off-by: Fabio Estevam 


Please consider applying this series to 2024.04.

It fixes a boot regression on imx8qxp and imx8qm.


Fine with me, Sean?



Acked-by: Sean Anderson 

Can you pick this up directly, Tom? Thanks.

Re: [PATCH] clk: Propagate clk_set_rate() if CLK_SET_PARENT_RATE present

[Sean Anderson]

On 3/20/24 10:02, Yang Xiwen wrote:

On 3/20/2024 2:42 AM, Sam Protsenko wrote:

On Thu, Mar 7, 2024 at 6:04 PM Sam Protsenko  wrote:

Sometimes clocks provided to a consumer might not have .set_rate
operation (like gate or mux clocks), but have CLK_SET_PARENT_RATE flag
set. In that case it's usually possible to find a parent up the tree
which is capable of setting the rate (div, pll, etc). Implement a simple
lookup procedure for such cases, to traverse the clock tree until
.set_rate capable parent is found, and use that parent to actually
change the rate. The search will stop once the first .set_rate capable
clock is found, which is usually enough to handle most cases.

Signed-off-by: Sam Protsenko 
---

Hi Lukasz, Sean, Tom,

If this patch looks good to you and there are no outstanding comments,
can you please apply it? It's needed as a part of eMMC enablement for
E850-96 board, as eMMC gate (leaf) clock is specified as ciu clock in
dts, which requires further clock rate change propagation up to
divider clock.

Thanks!



Please be patient. The release cycle is quite long. My patch set for HiSilicon 
clk framework has been ignored for ~2months already.


Sorry, I've been busy IRL recently.

I will try to review patches in the backlog, but it might be a few more weeks.

--Sean

Re: [PATCH] cmd: bootm: add ELF file support

[Sean Anderson]

Hi Maxim,

On 3/19/24 12:10, Maxim Moskalets wrote:
> [You don't often get email from maximmo...@gmail.com. Learn why this is 
> important at 
> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2faka.ms%2fLearnAboutSenderIdentification&umid=d3728a4f-0e7b-4ded-94bc-5bd133b6deef&auth=d807158c60b7d2502abde8a2fc01f40662980862-7a67d937c0016d0d3292f79af55f0ce3d78f480b
>  ]
>
> Some operating systems (e.g. seL4) and embedded applications are ELF
> images. It is convenient to use FIT-images to implement trusted boot.
> Added "elf" image type for booting using bootm command.

Why not use objcopy to create a binary?

--Sean

> Signed-off-by: Maxim Moskalets 
> ---
>  boot/bootm_os.c  | 21 +
>  boot/image-fit.c |  3 ++-
>  boot/image.c |  3 +++
>  include/image.h  |  1 +
>  4 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/boot/bootm_os.c b/boot/bootm_os.c
> index ccde72d22c..71bfb34926 100644
> --- a/boot/bootm_os.c
> +++ b/boot/bootm_os.c
> @@ -395,6 +395,24 @@ static int do_bootm_qnxelf(int flag, struct bootm_info 
> *bmi)
>  }
>  #endif
>
> +#if defined(CONFIG_CMD_ELF)
> +static int do_bootm_elf(int flag, struct bootm_info *bmi)
> +{
> +   struct bootm_headers *images = bmi->images;
> +   char *local_args[2] = {NULL};
> +   char str[19] = ""; /* "0x" + 16 digits + "\0" */
> +
> +   sprintf(str, "0x%lx", images->ep); /* write entry-point into string */
> +   str[18] = '\0';
> +   local_args[0] = bmi->argv[0];
> +   local_args[1] = str;/* and provide it via the arguments */
> +
> +   do_bootelf(NULL, 0, 2, local_args);
> +
> +   return 1;
> +}
> +#endif
> +
>  #ifdef CONFIG_INTEGRITY
>  static int do_bootm_integrity(int flag, struct bootm_info *bmi)
>  {
> @@ -536,6 +554,9 @@ static boot_os_fn *boot_os[] = {
>  #ifdef CONFIG_BOOTM_EFI
> [IH_OS_EFI] = do_bootm_efi,
>  #endif
> +#if defined(CONFIG_CMD_ELF)
> +   [IH_OS_ELF] = do_bootm_elf,
> +#endif
>  };
>
>  /* Allow for arch specific config before we boot */
> diff --git a/boot/image-fit.c b/boot/image-fit.c
> index 89e377563c..0419bef6d2 100644
> --- a/boot/image-fit.c
> +++ b/boot/image-fit.c
> @@ -2180,7 +2180,8 @@ int fit_image_load(struct bootm_headers *images, ulong 
> addr,
> fit_image_check_os(fit, noffset, IH_OS_TEE) ||
> fit_image_check_os(fit, noffset, IH_OS_OPENRTOS) ||
> fit_image_check_os(fit, noffset, IH_OS_EFI) ||
> -   fit_image_check_os(fit, noffset, IH_OS_VXWORKS);
> +   fit_image_check_os(fit, noffset, IH_OS_VXWORKS) ||
> +   fit_image_check_os(fit, noffset, IH_OS_ELF);
>
> /*
>  * If either of the checks fail, we should report an error, but
> diff --git a/boot/image.c b/boot/image.c
> index 073931cd7a..5b88d6808c 100644
> --- a/boot/image.c
> +++ b/boot/image.c
> @@ -134,6 +134,9 @@ static const table_entry_t uimage_os[] = {
>  #endif
> {   IH_OS_OPENSBI,  "opensbi",  "RISC-V OpenSBI",   },
> {   IH_OS_EFI,  "efi",  "EFI Firmware" },
> +#ifdef CONFIG_CMD_ELF
> +   {   IH_OS_ELF,  "elf",  "ELF Image" },
> +#endif
>
> {   -1, "", "", },
>  };
> diff --git a/include/image.h b/include/image.h
> index 21de70f0c9..9a40bca22c 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -100,6 +100,7 @@ enum {
> IH_OS_TEE,  /* Trusted Execution Environment */
> IH_OS_OPENSBI,  /* RISC-V OpenSBI */
> IH_OS_EFI,  /* EFI Firmware (e.g. GRUB2) */
> +   IH_OS_ELF,  /* ELF Image (e.g. seL4) */
>
> IH_OS_COUNT,
>  };
> --
> 2.39.2
>


[Embedded World 2024, SECO 
SpA]

Re: [PATCH v2] cmd: nand: Add support to print the manufacturer, model and size

[Sean Anderson]

On 3/18/24 09:42, Mihai Sain wrote:

Add support to nand info for printing the manufacturer,model and size.
The manufacturer and model are printed only for ONFI flashes.

U-Boot> nand info

Device 0: nand0, sector size 256 KiB
   Manufacturer  MACRONIX
   Model MX30LF4G28AD
   Device size512 MiB
   Page size 4096 b
   OOB size   256 b
   Erase size  262144 b
   ecc strength 8 bits
   ecc step size  512 b
   subpagesize   4096 b
   options   0x4200
   bbt options   0x00028000

Signed-off-by: Mihai Sain 

Changes in v2:
--
* use #ifdef directive for ONFI flashes.
* use unsigned int for chipsize.
---
  cmd/nand.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/cmd/nand.c b/cmd/nand.c
index fe834c4ac5..5773246d64 100644
--- a/cmd/nand.c
+++ b/cmd/nand.c
@@ -418,6 +418,11 @@ static void nand_print_and_set_info(int idx)
printf("%dx ", chip->numchips);
printf("%s, sector size %u KiB\n",
   mtd->name, mtd->erasesize >> 10);
+#ifdef CONFIG_SYS_NAND_ONFI_DETECTION
+   printf("  Manufacturer  %s \n", chip->onfi_params.manufacturer);
+   printf("  Model %s \n", chip->onfi_params.model);
+#endif
+   printf("  Device size   %8u MiB\n", (unsigned int)(chip->chipsize >> 
20));
printf("  Page size %8d b\n", mtd->writesize);
printf("  OOB size  %8d b\n", mtd->oobsize);
printf("  Erase size%8d b\n", mtd->erasesize);


Can you refactor the logic out of the end of nand_detect as a separate
function and use that instead? That will cover more cases (e.g. JEDEC).

--Sean

Re: [PATCH v5] misc: fs-loader: Use fw_storage_interface instead of storage_interface

[Sean Anderson]

Hi Danish,

On 2/27/24 05:26, MD Danish Anwar wrote:

On 09/02/24 3:38 pm, MD Danish Anwar wrote:

The fs-loader driver reads env storage_interface and uses it to load
firmware file into memory using the medium set by env. Update the driver
to use env fw_storage_interface as this variable is only used to load
firmwares. This is to keep all variables used by fs-loader driver with
'fw_' prefix. All other variables have 'fw_' prefix except for
storage_interface.

The env storage_interface will act as fallback so that the
existing implementations do not break.

Also update the FS Loader documentation accordingly.

Signed-off-by: MD Danish Anwar 
---


Hi Tom / Sean, can you please pick this patch if there is no pending
comments to address.



Sorry, I forgot to respond to this earlier.

To be honest, I'm not really convinced. We have plenty of environmental
variables which are inconsistent (e.g. ethaddr, eth2addr, eth3addr) and it
doesn't cause any issues. While fixing code has no cost, the environment
is an ABI which we can't break. So we'd have to support both of these
variables forever. I'm not really a fan of doing that without good reason,
and I think aesthetics of the variable name isn't really compelling.

--Sean

Re: [PATCH] clk: Revise help text for clk_get_parent_rate()

[Sean Anderson]

On 2/23/24 07:01, Alexander Dahl wrote:

The function returns the rate of the parent clock, the previous text
made no sense at all.

Fixes: 4aa78300a025 ("dm: clk: Define clk_get_parent_rate() for clk operations")
Signed-off-by: Alexander Dahl 
---
  include/clk.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/clk.h b/include/clk.h
index af23e4f3475..045e923a529 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -444,7 +444,7 @@ ulong clk_get_rate(struct clk *clk);
  struct clk *clk_get_parent(struct clk *clk);
  
  /**

- * clk_get_parent_rate() - Get parent of current clock rate.
+ * clk_get_parent_rate() - Get rate of current clock's parent.
   * @clk:  A clock struct that was previously successfully requested by
   *clk_request/get_by_*().
   *

base-commit: 7bb761c42d75b2ebacc7190a76cc5385cbba1045


Reviewed-by: Sean Anderson 

Re: [PATCH] Check curve_name for null to avoid crash

[Sean Anderson]

On 2/22/24 17:18, Bob Wolff wrote:

If mixed rsa and ecdsa keys are specified in dtsi, an rsa key can be sent
into the ecdsa verify. Without the ecdsa,curve property, this function will
crash due to lack of checking the null pointer return.


nit: there should be a blank line here


Signed-off-by: Bob Wolff 
---

  lib/ecdsa/ecdsa-verify.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/lib/ecdsa/ecdsa-verify.c b/lib/ecdsa/ecdsa-verify.c
index 0601700c4f..4d1835b598 100644
--- a/lib/ecdsa/ecdsa-verify.c
+++ b/lib/ecdsa/ecdsa-verify.c
@@ -31,6 +31,11 @@ static int fdt_get_key(struct ecdsa_public_key *key, const 
void *fdt, int node)
int x_len, y_len;
  
  	key->curve_name = fdt_getprop(fdt, node, "ecdsa,curve", NULL);

+   if (!key->curve_name) {
+   debug("Error: ecdsa cannot get 'ecdsa,curve' property from key. 
Likely not an ecdsa key.\n");
+   return -ENOMSG;
+   }
+
key->size_bits = ecdsa_key_size(key->curve_name);
if (key->size_bits == 0) {
debug("Unknown ECDSA curve '%s'", key->curve_name);


Reviewed-by: Sean Anderson 

[RESEND PATCH v2] arm64: zynqmp: Support semihosting boot method

[Sean Anderson]

Currently, when we boot from JTAG we try to boot U-Boot from RAM.
However, this is a bit tricky to time, since the debugger has to wait
for SPL to initialize RAM before it can load U-Boot. This can result in
long waits, since occasionally initializing RAM (and other things in
psu_init) takes a long time to complete and the debugger must wait for
this worst case.

Support semihosting if it is enabled, as it lets U-Boot tell the
debugger when we are ready for the image. This means we don't have to
wait any more than necessary. We don't change the default config to
ensure we don't break compatibility with existing debuggers that don't
expect us to hit semihosting breakpoints.

Signed-off-by: Sean Anderson 
---
I'm resending this from my linux.dev email as my regular email is
mangling my patches.

Changes in v2:
- Fix typo in subject

 arch/arm/mach-zynqmp/spl.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
index a0f35f36faa..5af735aa5ce 100644
--- a/arch/arm/mach-zynqmp/spl.c
+++ b/arch/arm/mach-zynqmp/spl.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -66,6 +67,11 @@ void spl_board_init(void)
 }
 #endif
 
+static u32 jtag_boot_device(void)
+{
+   return semihosting_enabled() ? BOOT_DEVICE_SMH : BOOT_DEVICE_RAM;
+}
+
 void board_boot_order(u32 *spl_boot_list)
 {
spl_boot_list[0] = spl_boot_device();
@@ -75,7 +81,7 @@ void board_boot_order(u32 *spl_boot_list)
if (spl_boot_list[0] == BOOT_DEVICE_MMC2)
spl_boot_list[1] = BOOT_DEVICE_MMC1;
 
-   spl_boot_list[2] = BOOT_DEVICE_RAM;
+   spl_boot_list[2] = jtag_boot_device();
 }
 
 u32 spl_boot_device(void)
@@ -97,7 +103,7 @@ u32 spl_boot_device(void)
 
switch (bootmode) {
case JTAG_MODE:
-   return BOOT_DEVICE_RAM;
+   return jtag_boot_device();
 #ifdef CONFIG_SPL_MMC
case SD_MODE1:
case SD1_LSHFT_MODE: /* not working on silicon v1 */
-- 
2.35.1.1320.gc452695387.dirty

Re: [PATCH v2] arm64: zynqmp: Support semihosting boot method

[Sean Anderson]

On 2/22/24 14:36, Fabio Estevam wrote:
> On Thu, Feb 22, 2024 at 1:53 PM Sean Anderson  wrote:
>>
>> From: Sean Anderson 
> ...
>> Signed-off-by: Sean Anderson 
>> ---
>> I'm resending this from my linux.dev email as my regular email is
>> mangling my patches.
> 
> That's OK, but you need to make sure that the From and Signed-off-by
> fields match.
> 
> Otherwise, checkpatch complains.

Yeah, I thought I had fixed the authors, but I guess not.

--Sean

Re: [PATCH] arm64: zynqmp: Support semhosting boot method

[Sean Anderson]

On 2/22/24 05:34, Michal Simek wrote:
>
>
> On 2/20/24 20:30, Sean Anderson wrote:
>> On 2/20/24 14:18, Michal Simek wrote:
>>>
>>>
>>> On 2/20/24 19:43, Sean Anderson wrote:
>>>> On 2/20/24 13:24, Michal Simek wrote:
>>>>>
>>>>>
>>>>> On 2/16/24 17:09, Sean Anderson wrote:
>>>>>> On 2/16/24 11:03, Sean Anderson wrote:
>>>>>>> On 2/16/24 10:06, Michal Simek wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2/16/24 14:48, Michal Simek wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2/15/24 20:31, Sean Anderson wrote:
>>>>>>>>>> On 2/15/24 14:08, Michal Simek wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 2/15/24 18:19, Sean Anderson wrote:
>>>>>>>>>>>> Currently, when we boot from JTAG we try to boot U-Boot from RAM.
>>>>>>>>>>>> However, this is a bit tricky to time, since the debugger has to 
>>>>>>>>>>>> wait
>>>>>>>>>>>> for SPL to initialize RAM before it can load U-Boot. This can 
>>>>>>>>>>>> result in
>>>>>>>>>>>> long waits, since occasionally initializing RAM (and other things 
>>>>>>>>>>>> in
>>>>>>>>>>>> psu_init) takes a long time to complete and the debugger must wait 
>>>>>>>>>>>> for
>>>>>>>>>>>> this worst case.
>>>>>>>>>>>>
>>>>>>>>>>>> Support semihosting if it is enabled, as it lets U-Boot tell the
>>>>>>>>>>>> debugger when we are ready for the image. This means we don't have 
>>>>>>>>>>>> to
>>>>>>>>>>>> wait any more than necessary. We don't change the default config to
>>>>>>>>>>>> ensure we don't break compatibility with existing debuggers that 
>>>>>>>>>>>> don't
>>>>>>>>>>>> expect us to hit semihosting breakpoints.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Sean Anderson 
>>>>>>>>>>>> ---
>>>>>>>>>>>>
>>>>>>>>>>>>   arch/arm/mach-zynqmp/spl.c | 10 --
>>>>>>>>>>>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/arch/arm/mach-zynqmp/spl.c 
>>>>>>>>>>>> b/arch/arm/mach-zynqmp/spl.c
>>>>>>>>>>>> index a0f35f36faa..5af735aa5ce 100644
>>>>>>>>>>>> --- a/arch/arm/mach-zynqmp/spl.c
>>>>>>>>>>>> +++ b/arch/arm/mach-zynqmp/spl.c
>>>>>>>>>>>> @@ -9,6 +9,7 @@
>>>>>>>>>>>>   #include 
>>>>>>>>>>>>   #include 
>>>>>>>>>>>>   #include 
>>>>>>>>>>>> +#include 
>>>>>>>>>>>>   #include 
>>>>>>>>>>>>   #include 
>>>>>>>>>>>>
>>>>>>>>>>>> @@ -66,6 +67,11 @@ void spl_board_init(void)
>>>>>>>>>>>>   }
>>>>>>>>>>>>   #endif
>>>>>>>>>>>>
>>>>>>>>>>>> +static u32 jtag_boot_device(void)
>>>>>>>>>>>> +{
>>>>>>>>>>>> +   return semihosting_enabled() ? BOOT_DEVICE_SMH : 
>>>>>>>>>>>> BOOT_DEVICE_RAM;
>>>>>>>>>>>> +}
>>>>>>>>>>>> +
>>>>>>>>>>>>   void board_boot_order(u32 *spl_boot_list)
>>>>>>>>>>>>   {
>>>>>>>>>>>>  spl_boot_list[0] = spl_boot_device();
>&

[PATCH v2] arm64: zynqmp: Support semihosting boot method

[Sean Anderson]

From: Sean Anderson 

Currently, when we boot from JTAG we try to boot U-Boot from RAM.
However, this is a bit tricky to time, since the debugger has to wait
for SPL to initialize RAM before it can load U-Boot. This can result in
long waits, since occasionally initializing RAM (and other things in
psu_init) takes a long time to complete and the debugger must wait for
this worst case.

Support semihosting if it is enabled, as it lets U-Boot tell the
debugger when we are ready for the image. This means we don't have to
wait any more than necessary. We don't change the default config to
ensure we don't break compatibility with existing debuggers that don't
expect us to hit semihosting breakpoints.

Signed-off-by: Sean Anderson 
---
I'm resending this from my linux.dev email as my regular email is
mangling my patches.

Changes in v2:
- Fix typo in subject

 arch/arm/mach-zynqmp/spl.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
index a0f35f36faa..5af735aa5ce 100644
--- a/arch/arm/mach-zynqmp/spl.c
+++ b/arch/arm/mach-zynqmp/spl.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -66,6 +67,11 @@ void spl_board_init(void)
 }
 #endif
 
+static u32 jtag_boot_device(void)
+{
+   return semihosting_enabled() ? BOOT_DEVICE_SMH : BOOT_DEVICE_RAM;
+}
+
 void board_boot_order(u32 *spl_boot_list)
 {
spl_boot_list[0] = spl_boot_device();
@@ -75,7 +81,7 @@ void board_boot_order(u32 *spl_boot_list)
if (spl_boot_list[0] == BOOT_DEVICE_MMC2)
spl_boot_list[1] = BOOT_DEVICE_MMC1;
 
-   spl_boot_list[2] = BOOT_DEVICE_RAM;
+   spl_boot_list[2] = jtag_boot_device();
 }
 
 u32 spl_boot_device(void)
@@ -97,7 +103,7 @@ u32 spl_boot_device(void)
 
switch (bootmode) {
case JTAG_MODE:
-   return BOOT_DEVICE_RAM;
+   return jtag_boot_device();
 #ifdef CONFIG_SPL_MMC
case SD_MODE1:
case SD1_LSHFT_MODE: /* not working on silicon v1 */
-- 
2.35.1.1320.gc452695387.dirty

Re: [PATCH] ecdsa: Avoid null pointer crash in ecdsa-verify due to absent property

[Sean Anderson]

Hi Bob,

On 2/21/24 19:27, Bob Wolff wrote:

If mixed rsa and ecdsa keys are specified in
dtsi, an rsa key can be sent into the ecdsa
verify. Without the ecdsa,curve property, this
function will crash due to lack of checking
the null pointer return.


You can wrap commit messages at 75 characters


Signed-off-by: Bob Wolff 
---

  lib/ecdsa/ecdsa-verify.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/lib/ecdsa/ecdsa-verify.c b/lib/ecdsa/ecdsa-verify.c
index 0601700c4f..01ffc3477c 100644
--- a/lib/ecdsa/ecdsa-verify.c
+++ b/lib/ecdsa/ecdsa-verify.c
@@ -31,6 +31,11 @@ static int fdt_get_key(struct ecdsa_public_key *key,
const void *fdt, int node)
   int x_len, y_len;

   key->curve_name = fdt_getprop(fdt, node, "ecdsa,curve", NULL);
+ if (!key->curve_name) {
+ printf("Error: ecdsa cannot get 'ecdsa,curve' property from key. Likely
not an ecdsa key.\n");


this should probably be a debug (like the below message)


+ return -ENOMSG;
+ }
+


and it looks like something ate your indentation

--Sean


   key->size_bits = ecdsa_key_size(key->curve_name);
   if (key->size_bits == 0) {
   debug("Unknown ECDSA curve '%s'", key->curve_name);
--
2.39.3 (Apple Git-145)

Re: [PATCH] arm64: zynqmp: Support semhosting boot method

[Sean Anderson]

On 2/20/24 14:18, Michal Simek wrote:
>
>
> On 2/20/24 19:43, Sean Anderson wrote:
>> On 2/20/24 13:24, Michal Simek wrote:
>>>
>>>
>>> On 2/16/24 17:09, Sean Anderson wrote:
>>>> On 2/16/24 11:03, Sean Anderson wrote:
>>>>> On 2/16/24 10:06, Michal Simek wrote:
>>>>>>
>>>>>>
>>>>>> On 2/16/24 14:48, Michal Simek wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2/15/24 20:31, Sean Anderson wrote:
>>>>>>>> On 2/15/24 14:08, Michal Simek wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2/15/24 18:19, Sean Anderson wrote:
>>>>>>>>>> Currently, when we boot from JTAG we try to boot U-Boot from RAM.
>>>>>>>>>> However, this is a bit tricky to time, since the debugger has to wait
>>>>>>>>>> for SPL to initialize RAM before it can load U-Boot. This can result 
>>>>>>>>>> in
>>>>>>>>>> long waits, since occasionally initializing RAM (and other things in
>>>>>>>>>> psu_init) takes a long time to complete and the debugger must wait 
>>>>>>>>>> for
>>>>>>>>>> this worst case.
>>>>>>>>>>
>>>>>>>>>> Support semihosting if it is enabled, as it lets U-Boot tell the
>>>>>>>>>> debugger when we are ready for the image. This means we don't have to
>>>>>>>>>> wait any more than necessary. We don't change the default config to
>>>>>>>>>> ensure we don't break compatibility with existing debuggers that 
>>>>>>>>>> don't
>>>>>>>>>> expect us to hit semihosting breakpoints.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Sean Anderson 
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>>  arch/arm/mach-zynqmp/spl.c | 10 --
>>>>>>>>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
>>>>>>>>>> index a0f35f36faa..5af735aa5ce 100644
>>>>>>>>>> --- a/arch/arm/mach-zynqmp/spl.c
>>>>>>>>>> +++ b/arch/arm/mach-zynqmp/spl.c
>>>>>>>>>> @@ -9,6 +9,7 @@
>>>>>>>>>>  #include 
>>>>>>>>>>  #include 
>>>>>>>>>>  #include 
>>>>>>>>>> +#include 
>>>>>>>>>>  #include 
>>>>>>>>>>  #include 
>>>>>>>>>>
>>>>>>>>>> @@ -66,6 +67,11 @@ void spl_board_init(void)
>>>>>>>>>>  }
>>>>>>>>>>  #endif
>>>>>>>>>>
>>>>>>>>>> +static u32 jtag_boot_device(void)
>>>>>>>>>> +{
>>>>>>>>>> +   return semihosting_enabled() ? BOOT_DEVICE_SMH : 
>>>>>>>>>> BOOT_DEVICE_RAM;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>>  void board_boot_order(u32 *spl_boot_list)
>>>>>>>>>>  {
>>>>>>>>>> spl_boot_list[0] = spl_boot_device();
>>>>>>>>>> @@ -75,7 +81,7 @@ void board_boot_order(u32 *spl_boot_list)
>>>>>>>>>> if (spl_boot_list[0] == BOOT_DEVICE_MMC2)
>>>>>>>>>> spl_boot_list[1] = BOOT_DEVICE_MMC1;
>>>>>>>>>>
>>>>>>>>>> -   spl_boot_list[2] = BOOT_DEVICE_RAM;
>>>>>>>>>> +   spl_boot_list[2] = jtag_boot_device();
>>>>>>>>>>  }
>>>>>>>>>>
>>>>>>>>>>  u32 spl_boot_device(void)
>>>>>>>>>> @@ -97,7 +103,7 @@ u32 spl_boot_device(void)
>>>>>>>>>>
>>>>>>>>>> switch (bootmode) {
>>&

Re: [PATCH] arm64: zynqmp: Support semhosting boot method

[Sean Anderson]

On 2/20/24 13:24, Michal Simek wrote:
>
>
> On 2/16/24 17:09, Sean Anderson wrote:
>> On 2/16/24 11:03, Sean Anderson wrote:
>>> On 2/16/24 10:06, Michal Simek wrote:
>>>>
>>>>
>>>> On 2/16/24 14:48, Michal Simek wrote:
>>>>>
>>>>>
>>>>> On 2/15/24 20:31, Sean Anderson wrote:
>>>>>> On 2/15/24 14:08, Michal Simek wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2/15/24 18:19, Sean Anderson wrote:
>>>>>>>> Currently, when we boot from JTAG we try to boot U-Boot from RAM.
>>>>>>>> However, this is a bit tricky to time, since the debugger has to wait
>>>>>>>> for SPL to initialize RAM before it can load U-Boot. This can result in
>>>>>>>> long waits, since occasionally initializing RAM (and other things in
>>>>>>>> psu_init) takes a long time to complete and the debugger must wait for
>>>>>>>> this worst case.
>>>>>>>>
>>>>>>>> Support semihosting if it is enabled, as it lets U-Boot tell the
>>>>>>>> debugger when we are ready for the image. This means we don't have to
>>>>>>>> wait any more than necessary. We don't change the default config to
>>>>>>>> ensure we don't break compatibility with existing debuggers that don't
>>>>>>>> expect us to hit semihosting breakpoints.
>>>>>>>>
>>>>>>>> Signed-off-by: Sean Anderson 
>>>>>>>> ---
>>>>>>>>
>>>>>>>> arch/arm/mach-zynqmp/spl.c | 10 --
>>>>>>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
>>>>>>>> index a0f35f36faa..5af735aa5ce 100644
>>>>>>>> --- a/arch/arm/mach-zynqmp/spl.c
>>>>>>>> +++ b/arch/arm/mach-zynqmp/spl.c
>>>>>>>> @@ -9,6 +9,7 @@
>>>>>>>> #include 
>>>>>>>> #include 
>>>>>>>> #include 
>>>>>>>> +#include 
>>>>>>>> #include 
>>>>>>>> #include 
>>>>>>>>
>>>>>>>> @@ -66,6 +67,11 @@ void spl_board_init(void)
>>>>>>>> }
>>>>>>>> #endif
>>>>>>>>
>>>>>>>> +static u32 jtag_boot_device(void)
>>>>>>>> +{
>>>>>>>> +   return semihosting_enabled() ? BOOT_DEVICE_SMH : 
>>>>>>>> BOOT_DEVICE_RAM;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> void board_boot_order(u32 *spl_boot_list)
>>>>>>>> {
>>>>>>>>spl_boot_list[0] = spl_boot_device();
>>>>>>>> @@ -75,7 +81,7 @@ void board_boot_order(u32 *spl_boot_list)
>>>>>>>>if (spl_boot_list[0] == BOOT_DEVICE_MMC2)
>>>>>>>>spl_boot_list[1] = BOOT_DEVICE_MMC1;
>>>>>>>>
>>>>>>>> -   spl_boot_list[2] = BOOT_DEVICE_RAM;
>>>>>>>> +   spl_boot_list[2] = jtag_boot_device();
>>>>>>>> }
>>>>>>>>
>>>>>>>> u32 spl_boot_device(void)
>>>>>>>> @@ -97,7 +103,7 @@ u32 spl_boot_device(void)
>>>>>>>>
>>>>>>>>switch (bootmode) {
>>>>>>>>case JTAG_MODE:
>>>>>>>> -   return BOOT_DEVICE_RAM;
>>>>>>>> +   return jtag_boot_device();
>>>>>>>> #ifdef CONFIG_SPL_MMC
>>>>>>>>case SD_MODE1:
>>>>>>>>case SD1_LSHFT_MODE: /* not working on silicon v1 */
>>>>>>>
>>>>>>> Good timing. Can you please tell me how to test this? What's the setup?
>>>>>>> Which debugger are you using?
>>>>>>
>>>>>> I am using OpenOCD with the patches at 
>>>>>> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2

Re: HABv4 with SPL and u-boot-dtb.img on i.MX6

[Sean Anderson]

On 2/20/24 04:50, Benjamin Lemouzy wrote:
> Hello,
>
> I'm trying to make secure boot work on i.MX6 SABRE with SPL and 
> u-boot-dtb.img files and I'm not sure how to do it.
>
> I'm using the U-Boot vanilla master branch (2024.04-rc2) with the following 
> configuration:
>
> # Remove some stuff to not exceed file size limit
> $ cat <> configs/mx6sabresd_defconfig
> CONFIG_BOOTMETH_EFILOADER=n
> CONFIG_CMD_NET=n
> CONFIG_NET=n
> EOF
>
> # Enable secure boot
> $ cat <> configs/mx6sabresd_defconfig
> CONFIG_IMX_HAB=y
> CONFIG_SPL_LOAD_FIT_ADDRESS=0x1800
> EOF
>
> $ make ARCH=arm O=build mx6sabresd_defconfig
>
> $ make ARCH=arm O=build
>
> I have no issue to generate a working SPL-signed file following 
> doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt instructions.
>
> doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt only gives instructions to 
> sign u-boot-ivt.img but this file doesn't contain device trees listed in 
> CONFIG_OF_LIST as u-boot-dtb.img does and I need them.
>
>
>
> NXP AN4581 lists 2 possible formats to sign additional images:
>
> - Image format:
>
> --- +-+ <-- *load_address
> ^   | |
> |   | |
> |   |  Image data |
>  Signed |   | |
>   Data  |   | |
> |   +-+
> |   |Padding Next Boundary|
> |   +-+ <-- *ivt
> v   | Image Vector Table  |
> --- +-+ <-- *csf
> | |
> | Command Sequence File (CSF) |
> | |
> +-+
> | Padding (optional)  |
> +-+
>
> - FIT image format:
>
> --- +-+ ---
> ^   | |^
> |   | ||
> |   |   FDT FIT   ||
> |   | ||
> Signed data |   | ||
> |   +-+|
> |   |Padding Next Boundary||
> |   +-+|
> v   | Image Vector Table  ||
> --- +-+| FIT image
> | ||
> | Command Sequence File (CSF) ||
> | ||
> +-+|
> | Padding (optional)  ||
> --- +-+|
> ^   | ||
> Signed data |   |   U-Boot||
> v   | |v
> --- +-+ ---
>
> And as u-boot-dtb.img is a FIT image, I probably have to use the FIT image 
> format, right?
>
>
>
> I manually craft the signed FIT image using 
> doc/imx/habv4/csf_examples/mx8m/csf.sh as reference and everything looks fine:
>
> U-Boot SPL 2024.04-rc2-00025-g9e00b6993f-dirty (Feb 19 2024 - 13:17:31 
> +0100)
> >>SPL: board_init_r()
> spl_init
> Trying to boot from MMC1
> fit read offset 11400, size=12800, dst=1800, count=12800
> spl_load_simple_fit_fix_load: ivt: 18001000 offset: 1000 size: 3060
> spl_load_simple_fit_fix_load: ivt self: 18001000
> hab fuse not enabled
>
> Authenticate image from DDR location 0x1800...
>
> ivt_offset = 0x1000, ivt addr = 0x18001000
> ivt entry = 0x1800, dcd = 0x, csf = 0x18001020
> Dumping IVT
> .. @
>  ...
> Dumping CSF Header
> ..PC...P
> 
> ...<
> ...8
>
> Calling authenticate_image in ROM
> ivt_offset = 0x1000
> start = 0x1800
> bytes = 0x3060
> firmware: 'firmware-1'
> External data: dst=1780, offset=3060, size=86138
> Image OS is U-Boot
> fdt: 'fdt-1'
> Can't get 'load' property from FIT 0x1800, node: offset 464, name 
> fdt-1 (FDT_ERR_NOTFOUND)
> External data: dst=17886140, offset=89198, size=ac00
> Can't get 'entry' property from FIT 0x1800, node: offset 464, name 
> fdt-1 (FDT_ERR_NOTFOUND)
> loadables: 'firmware-1'
> no string for index 1
> Jumping to U-Boot...
> SPL malloc() used 0x0 byt

Re: [PATCH] arm64: zynqmp: Support semhosting boot method

[Sean Anderson]

On 2/16/24 11:03, Sean Anderson wrote:
> On 2/16/24 10:06, Michal Simek wrote:
>>
>>
>> On 2/16/24 14:48, Michal Simek wrote:
>>>
>>>
>>> On 2/15/24 20:31, Sean Anderson wrote:
>>>> On 2/15/24 14:08, Michal Simek wrote:
>>>>>
>>>>>
>>>>> On 2/15/24 18:19, Sean Anderson wrote:
>>>>>> Currently, when we boot from JTAG we try to boot U-Boot from RAM.
>>>>>> However, this is a bit tricky to time, since the debugger has to wait
>>>>>> for SPL to initialize RAM before it can load U-Boot. This can result in
>>>>>> long waits, since occasionally initializing RAM (and other things in
>>>>>> psu_init) takes a long time to complete and the debugger must wait for
>>>>>> this worst case.
>>>>>>
>>>>>> Support semihosting if it is enabled, as it lets U-Boot tell the
>>>>>> debugger when we are ready for the image. This means we don't have to
>>>>>> wait any more than necessary. We don't change the default config to
>>>>>> ensure we don't break compatibility with existing debuggers that don't
>>>>>> expect us to hit semihosting breakpoints.
>>>>>>
>>>>>> Signed-off-by: Sean Anderson 
>>>>>> ---
>>>>>>
>>>>>>arch/arm/mach-zynqmp/spl.c | 10 --
>>>>>>1 file changed, 8 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
>>>>>> index a0f35f36faa..5af735aa5ce 100644
>>>>>> --- a/arch/arm/mach-zynqmp/spl.c
>>>>>> +++ b/arch/arm/mach-zynqmp/spl.c
>>>>>> @@ -9,6 +9,7 @@
>>>>>>#include 
>>>>>>#include 
>>>>>>#include 
>>>>>> +#include 
>>>>>>#include 
>>>>>>#include 
>>>>>>
>>>>>> @@ -66,6 +67,11 @@ void spl_board_init(void)
>>>>>>}
>>>>>>#endif
>>>>>>
>>>>>> +static u32 jtag_boot_device(void)
>>>>>> +{
>>>>>> +   return semihosting_enabled() ? BOOT_DEVICE_SMH : BOOT_DEVICE_RAM;
>>>>>> +}
>>>>>> +
>>>>>>void board_boot_order(u32 *spl_boot_list)
>>>>>>{
>>>>>>   spl_boot_list[0] = spl_boot_device();
>>>>>> @@ -75,7 +81,7 @@ void board_boot_order(u32 *spl_boot_list)
>>>>>>   if (spl_boot_list[0] == BOOT_DEVICE_MMC2)
>>>>>>   spl_boot_list[1] = BOOT_DEVICE_MMC1;
>>>>>>
>>>>>> -   spl_boot_list[2] = BOOT_DEVICE_RAM;
>>>>>> +   spl_boot_list[2] = jtag_boot_device();
>>>>>>}
>>>>>>
>>>>>>u32 spl_boot_device(void)
>>>>>> @@ -97,7 +103,7 @@ u32 spl_boot_device(void)
>>>>>>
>>>>>>   switch (bootmode) {
>>>>>>   case JTAG_MODE:
>>>>>> -   return BOOT_DEVICE_RAM;
>>>>>> +   return jtag_boot_device();
>>>>>>#ifdef CONFIG_SPL_MMC
>>>>>>   case SD_MODE1:
>>>>>>   case SD1_LSHFT_MODE: /* not working on silicon v1 */
>>>>>
>>>>> Good timing. Can you please tell me how to test this? What's the setup?
>>>>> Which debugger are you using?
>>>>
>>>> I am using OpenOCD with the patches at 
>>>> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2freview.openocd.org%2fc%2fopenocd%2f%2b%2f8133&umid=6e1be473-0b3f-4bc4-a4f0-403592e74baf&auth=d807158c60b7d2502abde8a2fc01f40662980862-afc15b07b0f91c910f832185958363d84f990a08
>>>>
>>>
>>> I am trying it on the top of the latest git but getting issue with event 
>>> block and no idea how to fix it.
>>>
>>> # sudo openocd -f 
>>> /usr/local/share/openocd/scripts/interface/ftdi/digilent_jtag_hs3.cfg -f 
>>> /usr/local/share/openocd/scripts/target/xilinx_zynqmp.cfg
>>> Open On-Chip Debugger 0.12.0+dev-01509-g6d288937cb2d (2024-02-16-12:22)
>>> Licensed under GNU GPL v2
>>> For bug reports, read
>>>  
>>> https://cas5-0-urlprotect.trendmicro.com

Re: [PATCH] arm64: zynqmp: Support semhosting boot method

[Sean Anderson]

On 2/16/24 10:06, Michal Simek wrote:
>
>
> On 2/16/24 14:48, Michal Simek wrote:
>>
>>
>> On 2/15/24 20:31, Sean Anderson wrote:
>>> On 2/15/24 14:08, Michal Simek wrote:
>>>>
>>>>
>>>> On 2/15/24 18:19, Sean Anderson wrote:
>>>>> Currently, when we boot from JTAG we try to boot U-Boot from RAM.
>>>>> However, this is a bit tricky to time, since the debugger has to wait
>>>>> for SPL to initialize RAM before it can load U-Boot. This can result in
>>>>> long waits, since occasionally initializing RAM (and other things in
>>>>> psu_init) takes a long time to complete and the debugger must wait for
>>>>> this worst case.
>>>>>
>>>>> Support semihosting if it is enabled, as it lets U-Boot tell the
>>>>> debugger when we are ready for the image. This means we don't have to
>>>>> wait any more than necessary. We don't change the default config to
>>>>> ensure we don't break compatibility with existing debuggers that don't
>>>>> expect us to hit semihosting breakpoints.
>>>>>
>>>>> Signed-off-by: Sean Anderson 
>>>>> ---
>>>>>
>>>>>arch/arm/mach-zynqmp/spl.c | 10 --
>>>>>1 file changed, 8 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
>>>>> index a0f35f36faa..5af735aa5ce 100644
>>>>> --- a/arch/arm/mach-zynqmp/spl.c
>>>>> +++ b/arch/arm/mach-zynqmp/spl.c
>>>>> @@ -9,6 +9,7 @@
>>>>>#include 
>>>>>#include 
>>>>>#include 
>>>>> +#include 
>>>>>#include 
>>>>>#include 
>>>>>
>>>>> @@ -66,6 +67,11 @@ void spl_board_init(void)
>>>>>}
>>>>>#endif
>>>>>
>>>>> +static u32 jtag_boot_device(void)
>>>>> +{
>>>>> +   return semihosting_enabled() ? BOOT_DEVICE_SMH : BOOT_DEVICE_RAM;
>>>>> +}
>>>>> +
>>>>>void board_boot_order(u32 *spl_boot_list)
>>>>>{
>>>>>   spl_boot_list[0] = spl_boot_device();
>>>>> @@ -75,7 +81,7 @@ void board_boot_order(u32 *spl_boot_list)
>>>>>   if (spl_boot_list[0] == BOOT_DEVICE_MMC2)
>>>>>   spl_boot_list[1] = BOOT_DEVICE_MMC1;
>>>>>
>>>>> -   spl_boot_list[2] = BOOT_DEVICE_RAM;
>>>>> +   spl_boot_list[2] = jtag_boot_device();
>>>>>}
>>>>>
>>>>>u32 spl_boot_device(void)
>>>>> @@ -97,7 +103,7 @@ u32 spl_boot_device(void)
>>>>>
>>>>>   switch (bootmode) {
>>>>>   case JTAG_MODE:
>>>>> -   return BOOT_DEVICE_RAM;
>>>>> +   return jtag_boot_device();
>>>>>#ifdef CONFIG_SPL_MMC
>>>>>   case SD_MODE1:
>>>>>   case SD1_LSHFT_MODE: /* not working on silicon v1 */
>>>>
>>>> Good timing. Can you please tell me how to test this? What's the setup?
>>>> Which debugger are you using?
>>>
>>> I am using OpenOCD with the patches at 
>>> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2freview.openocd.org%2fc%2fopenocd%2f%2b%2f8133&umid=6e1be473-0b3f-4bc4-a4f0-403592e74baf&auth=d807158c60b7d2502abde8a2fc01f40662980862-afc15b07b0f91c910f832185958363d84f990a08
>>>
>>
>> I am trying it on the top of the latest git but getting issue with event 
>> block and no idea how to fix it.
>>
>> # sudo openocd -f 
>> /usr/local/share/openocd/scripts/interface/ftdi/digilent_jtag_hs3.cfg -f 
>> /usr/local/share/openocd/scripts/target/xilinx_zynqmp.cfg
>> Open On-Chip Debugger 0.12.0+dev-01509-g6d288937cb2d (2024-02-16-12:22)
>> Licensed under GNU GPL v2
>> For bug reports, read
>>  
>> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=http%3a%2f%2fopenocd.org%2fdoc%2fdoxygen%2fbugs.html&umid=6e1be473-0b3f-4bc4-a4f0-403592e74baf&auth=d807158c60b7d2502abde8a2fc01f40662980862-f501ab9aa5516ff666e387e53598fd624398f1bc
>> Info : auto-selecting first available session transport "jtag". To override 
>> use 'transport select '.
>> wrong # args: should be "-eve

Re: [PATCH] arm64: zynqmp: Support semhosting boot method

[Sean Anderson]

On 2/15/24 14:31, Sean Anderson wrote:
> On 2/15/24 14:08, Michal Simek wrote:
>>
>>
>> On 2/15/24 18:19, Sean Anderson wrote:
>>> Currently, when we boot from JTAG we try to boot U-Boot from RAM.
>>> However, this is a bit tricky to time, since the debugger has to wait
>>> for SPL to initialize RAM before it can load U-Boot. This can result in
>>> long waits, since occasionally initializing RAM (and other things in
>>> psu_init) takes a long time to complete and the debugger must wait for
>>> this worst case.
>>>
>>> Support semihosting if it is enabled, as it lets U-Boot tell the
>>> debugger when we are ready for the image. This means we don't have to
>>> wait any more than necessary. We don't change the default config to
>>> ensure we don't break compatibility with existing debuggers that don't
>>> expect us to hit semihosting breakpoints.
>>>
>>> Signed-off-by: Sean Anderson 
>>> ---
>>>
>>>   arch/arm/mach-zynqmp/spl.c | 10 --
>>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
>>> index a0f35f36faa..5af735aa5ce 100644
>>> --- a/arch/arm/mach-zynqmp/spl.c
>>> +++ b/arch/arm/mach-zynqmp/spl.c
>>> @@ -9,6 +9,7 @@
>>>   #include 
>>>   #include 
>>>   #include 
>>> +#include 
>>>   #include 
>>>   #include 
>>>
>>> @@ -66,6 +67,11 @@ void spl_board_init(void)
>>>   }
>>>   #endif
>>>
>>> +static u32 jtag_boot_device(void)
>>> +{
>>> +   return semihosting_enabled() ? BOOT_DEVICE_SMH : BOOT_DEVICE_RAM;
>>> +}
>>> +
>>>   void board_boot_order(u32 *spl_boot_list)
>>>   {
>>>  spl_boot_list[0] = spl_boot_device();
>>> @@ -75,7 +81,7 @@ void board_boot_order(u32 *spl_boot_list)
>>>  if (spl_boot_list[0] == BOOT_DEVICE_MMC2)
>>>  spl_boot_list[1] = BOOT_DEVICE_MMC1;
>>>
>>> -   spl_boot_list[2] = BOOT_DEVICE_RAM;
>>> +   spl_boot_list[2] = jtag_boot_device();
>>>   }
>>>
>>>   u32 spl_boot_device(void)
>>> @@ -97,7 +103,7 @@ u32 spl_boot_device(void)
>>>
>>>  switch (bootmode) {
>>>  case JTAG_MODE:
>>> -   return BOOT_DEVICE_RAM;
>>> +   return jtag_boot_device();
>>>   #ifdef CONFIG_SPL_MMC
>>>  case SD_MODE1:
>>>  case SD1_LSHFT_MODE: /* not working on silicon v1 */
>>
>> Good timing. Can you please tell me how to test this? What's the setup?
>> Which debugger are you using?
> 
> I am using OpenOCD with the patches at 
> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2freview.openocd.org%2fc%2fopenocd%2f%2b%2f8133&umid=819f1021-60b3-42c6-ac85-f327c489da7c&auth=d807158c60b7d2502abde8a2fc01f40662980862-7485d72bf875c6ddb963725debf981fe8dd33731
> 
> My boot flow is
> 
> SPL -> ATF -> U-Boot (no FSBL)
> 
> This allows me to use a FIT which saves some time since I can
> compress the bitstream. Right now I am using
> xilinx_zynqmp_virt_defconfig with the following modifications:
> 
> CONFIG_SYS_LOAD_ADDR=0x3000
> # CONFIG_SPL_OS_BOOT is not set
> CONFIG_SPL_SEMIHOSTING=y
> 
> I also have CONFIG_XILINX_PS_INIT_FILE,
> CONFIG_ZYNQMP_SPL_PM_CFG_OBJ_FILE, and CONFIG_PMUFW_INIT_FILE defined as
> appropriate. I use the following FIT for U-Boot:
> 
> /dts-v1/;
> 
> / {
> description = "U-Boot and ATF";
> #address-cells = <1>;
> 
> images {
> atf {
> description = "Arm Trusted Firmware";
> data = /incbin/("arm-trusted-firmware.bin");
> type = "firmware";
> os = "arm-trusted-firmware";
> arch = "arm64";
> compression = "none";
> load = <0xfffea000>;
> entry = <0xfffea000>;
> };
> 
> fpga {
> description = "PL Bitstream";
> data = /incbin/("system.bit.gz");
> type = "fpga";
> arch = "arm64";
> compression = "gzip";
> load = <0x7c00>;
> compatible = "u-boot,fpga-legacy";
> };
> 
> u-boot {
> description = "U-Boot";
>

Re: [PATCH] arm64: zynqmp: Support semhosting boot method

[Sean Anderson]

On 2/15/24 14:08, Michal Simek wrote:
>
>
> On 2/15/24 18:19, Sean Anderson wrote:
>> Currently, when we boot from JTAG we try to boot U-Boot from RAM.
>> However, this is a bit tricky to time, since the debugger has to wait
>> for SPL to initialize RAM before it can load U-Boot. This can result in
>> long waits, since occasionally initializing RAM (and other things in
>> psu_init) takes a long time to complete and the debugger must wait for
>> this worst case.
>>
>> Support semihosting if it is enabled, as it lets U-Boot tell the
>> debugger when we are ready for the image. This means we don't have to
>> wait any more than necessary. We don't change the default config to
>> ensure we don't break compatibility with existing debuggers that don't
>> expect us to hit semihosting breakpoints.
>>
>> Signed-off-by: Sean Anderson 
>> ---
>>
>>   arch/arm/mach-zynqmp/spl.c | 10 --
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
>> index a0f35f36faa..5af735aa5ce 100644
>> --- a/arch/arm/mach-zynqmp/spl.c
>> +++ b/arch/arm/mach-zynqmp/spl.c
>> @@ -9,6 +9,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>   #include 
>>   #include 
>>
>> @@ -66,6 +67,11 @@ void spl_board_init(void)
>>   }
>>   #endif
>>
>> +static u32 jtag_boot_device(void)
>> +{
>> +   return semihosting_enabled() ? BOOT_DEVICE_SMH : BOOT_DEVICE_RAM;
>> +}
>> +
>>   void board_boot_order(u32 *spl_boot_list)
>>   {
>>  spl_boot_list[0] = spl_boot_device();
>> @@ -75,7 +81,7 @@ void board_boot_order(u32 *spl_boot_list)
>>  if (spl_boot_list[0] == BOOT_DEVICE_MMC2)
>>  spl_boot_list[1] = BOOT_DEVICE_MMC1;
>>
>> -   spl_boot_list[2] = BOOT_DEVICE_RAM;
>> +   spl_boot_list[2] = jtag_boot_device();
>>   }
>>
>>   u32 spl_boot_device(void)
>> @@ -97,7 +103,7 @@ u32 spl_boot_device(void)
>>
>>  switch (bootmode) {
>>  case JTAG_MODE:
>> -   return BOOT_DEVICE_RAM;
>> +   return jtag_boot_device();
>>   #ifdef CONFIG_SPL_MMC
>>  case SD_MODE1:
>>  case SD1_LSHFT_MODE: /* not working on silicon v1 */
>
> Good timing. Can you please tell me how to test this? What's the setup?
> Which debugger are you using?

I am using OpenOCD with the patches at 
https://review.openocd.org/c/openocd/+/8133

My boot flow is

SPL -> ATF -> U-Boot (no FSBL)

This allows me to use a FIT which saves some time since I can
compress the bitstream. Right now I am using
xilinx_zynqmp_virt_defconfig with the following modifications:

CONFIG_SYS_LOAD_ADDR=0x3000
# CONFIG_SPL_OS_BOOT is not set
CONFIG_SPL_SEMIHOSTING=y

I also have CONFIG_XILINX_PS_INIT_FILE,
CONFIG_ZYNQMP_SPL_PM_CFG_OBJ_FILE, and CONFIG_PMUFW_INIT_FILE defined as
appropriate. I use the following FIT for U-Boot:

/dts-v1/;

/ {
description = "U-Boot and ATF";
#address-cells = <1>;

images {
atf {
description = "Arm Trusted Firmware";
data = /incbin/("arm-trusted-firmware.bin");
type = "firmware";
os = "arm-trusted-firmware";
arch = "arm64";
compression = "none";
load = <0xfffea000>;
entry = <0xfffea000>;
};

fpga {
description = "PL Bitstream";
data = /incbin/("system.bit.gz");
type = "fpga";
arch = "arm64";
compression = "gzip";
load = <0x7c00>;
compatible = "u-boot,fpga-legacy";
};

u-boot {
description = "U-Boot";
data = /incbin/("u-boot-nodtb.bin");
type = "firmware";
os = "u-boot";
arch = "arm64";
compression = "none";
load = <0x0008>;
entry = <0x0008>;
};

fdt {
description = "U-Boot FDT";
data = /incbin/("u-boot.dtb");
type = "flat_dt";
arch = "arm64";
compression = "none";
hash-1 {
algo = "crc32";
};
};
};

configurations {
default = "conf";
conf {
description = "Boot ATF and U-Boot";
firmware = &quo

[PATCH] arm64: zynqmp: Support semhosting boot method

[Sean Anderson]

Currently, when we boot from JTAG we try to boot U-Boot from RAM.
However, this is a bit tricky to time, since the debugger has to wait
for SPL to initialize RAM before it can load U-Boot. This can result in
long waits, since occasionally initializing RAM (and other things in
psu_init) takes a long time to complete and the debugger must wait for
this worst case.

Support semihosting if it is enabled, as it lets U-Boot tell the
debugger when we are ready for the image. This means we don't have to
wait any more than necessary. We don't change the default config to
ensure we don't break compatibility with existing debuggers that don't
expect us to hit semihosting breakpoints.

Signed-off-by: Sean Anderson 
---

 arch/arm/mach-zynqmp/spl.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
index a0f35f36faa..5af735aa5ce 100644
--- a/arch/arm/mach-zynqmp/spl.c
+++ b/arch/arm/mach-zynqmp/spl.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 

@@ -66,6 +67,11 @@ void spl_board_init(void)
 }
 #endif

+static u32 jtag_boot_device(void)
+{
+   return semihosting_enabled() ? BOOT_DEVICE_SMH : BOOT_DEVICE_RAM;
+}
+
 void board_boot_order(u32 *spl_boot_list)
 {
spl_boot_list[0] = spl_boot_device();
@@ -75,7 +81,7 @@ void board_boot_order(u32 *spl_boot_list)
if (spl_boot_list[0] == BOOT_DEVICE_MMC2)
spl_boot_list[1] = BOOT_DEVICE_MMC1;

-   spl_boot_list[2] = BOOT_DEVICE_RAM;
+   spl_boot_list[2] = jtag_boot_device();
 }

 u32 spl_boot_device(void)
@@ -97,7 +103,7 @@ u32 spl_boot_device(void)

switch (bootmode) {
case JTAG_MODE:
-   return BOOT_DEVICE_RAM;
+   return jtag_boot_device();
 #ifdef CONFIG_SPL_MMC
case SD_MODE1:
case SD1_LSHFT_MODE: /* not working on silicon v1 */
--
2.35.1.1320.gc452695387.dirty


[Embedded World 2024, SECO 
SpA]<https://www.messe-ticket.de/Nuernberg/embeddedworld2024/Register/ew24517689>

[PATCH] boot: Only define checksum algos when the hashes are enabled

[Sean Anderson]

Don't define checksum algos when the underlying hashes are not enabled.
This allows disabling these hashes in SPL (or U-Boot).

Fixes: d16b38f4270 ("Add support for SHA384 and SHA512")
Fixes: 646257d1f40 ("rsa: add sha256-rsa2048 algorithm")
Signed-off-by: Sean Anderson 
---

 boot/image-sig.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/boot/image-sig.c b/boot/image-sig.c
index b5692d58b24..0421a61b040 100644
--- a/boot/image-sig.c
+++ b/boot/image-sig.c
@@ -17,6 +17,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define IMAGE_MAX_HASHED_NODES 100

 struct checksum_algo checksum_algos[] = {
+#if CONFIG_IS_ENABLED(SHA1)
{
.name = "sha1",
.checksum_len = SHA1_SUM_LEN,
@@ -24,6 +25,8 @@ struct checksum_algo checksum_algos[] = {
.der_prefix = sha1_der_prefix,
.calculate = hash_calculate,
},
+#endif
+#if CONFIG_IS_ENABLED(SHA256)
{
.name = "sha256",
.checksum_len = SHA256_SUM_LEN,
@@ -31,7 +34,8 @@ struct checksum_algo checksum_algos[] = {
.der_prefix = sha256_der_prefix,
.calculate = hash_calculate,
},
-#ifdef CONFIG_SHA384
+#endif
+#if CONFIG_IS_ENABLED(SHA384)
{
.name = "sha384",
.checksum_len = SHA384_SUM_LEN,
@@ -40,7 +44,7 @@ struct checksum_algo checksum_algos[] = {
.calculate = hash_calculate,
},
 #endif
-#ifdef CONFIG_SHA512
+#if CONFIG_IS_ENABLED(SHA512)
{
.name = "sha512",
.checksum_len = SHA512_SUM_LEN,
--
2.35.1.1320.gc452695387.dirty


[Embedded World 2024, SECO 
SpA]<https://www.messe-ticket.de/Nuernberg/embeddedworld2024/Register/ew24517689>

Re: [PATCH v4] misc: fs-loader: Use fw_storage_interface instead of storage_interface

[Sean Anderson]

On 1/30/24 01:26, MD Danish Anwar wrote:

The fs-loader driver reads env storage_interface and uses it to load
firmware file into memory using the medium set by env. Update the driver
to use env fw_storage_interface as this variable is only used to load
firmwares. The env storage_interface will act as fallback so that the
existing implementations do not break.

Also update the FS Loader documentation accordingly.


So why do you want to do this? I don't see what the point of renaming the
variable is, since you are not e.g. adding any new functionality, and we
have to pay for the rename in code size.

--Sean

Re: [PATCH v3 3/3] board: Add support for Conclusive WHLE-LS1088A

[Sean Anderson]

On 11/28/23 05:34, Artur Rojek wrote:
> Introduce support for Conclusive WHLE-LS1088A Single Board Computer.
> 
> Co-developed-by: Jakub Klama 
> Signed-off-by: Jakub Klama 
> Signed-off-by: Artur Rojek 
> ---
> 
> v3: new patch
> 
>  arch/arm/Kconfig  |  19 ++
>  arch/arm/dts/Makefile |   1 +
>  arch/arm/dts/fsl-ls1088a-whle-u-boot.dtsi |   8 +
>  arch/arm/dts/fsl-ls1088a-whle.dts | 235 ++
>  board/conclusive/whle-ls1088a/Kconfig |  29 ++
>  board/conclusive/whle-ls1088a/MAINTAINERS |  11 +
>  board/conclusive/whle-ls1088a/Makefile|   7 +
>  board/conclusive/whle-ls1088a/ddr.c   | 134 
>  board/conclusive/whle-ls1088a/ddr.h   |  47 +++
>  board/conclusive/whle-ls1088a/eth.c   |  13 +
>  board/conclusive/whle-ls1088a/whle-ls1088a.c  | 301 ++
>  .../conclusive/whle-ls1088a/whle-ls1088a.env  |  13 +
>  configs/whle_ls1088a_emmc_defconfig   |  84 +
>  configs/whle_ls1088a_qspi_defconfig   |  84 +
>  include/configs/whle_ls1088a.h|  92 ++
>  15 files changed, 1078 insertions(+)
>  create mode 100644 arch/arm/dts/fsl-ls1088a-whle-u-boot.dtsi
>  create mode 100644 arch/arm/dts/fsl-ls1088a-whle.dts
>  create mode 100644 board/conclusive/whle-ls1088a/Kconfig
>  create mode 100644 board/conclusive/whle-ls1088a/MAINTAINERS
>  create mode 100644 board/conclusive/whle-ls1088a/Makefile
>  create mode 100644 board/conclusive/whle-ls1088a/ddr.c
>  create mode 100644 board/conclusive/whle-ls1088a/ddr.h
>  create mode 100644 board/conclusive/whle-ls1088a/eth.c
>  create mode 100644 board/conclusive/whle-ls1088a/whle-ls1088a.c
>  create mode 100644 board/conclusive/whle-ls1088a/whle-ls1088a.env
>  create mode 100644 configs/whle_ls1088a_emmc_defconfig
>  create mode 100644 configs/whle_ls1088a_qspi_defconfig
>  create mode 100644 include/configs/whle_ls1088a.h
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 609571e6e421..cd53fcaac883 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1869,6 +1869,24 @@ config TARGET_WHLE_LS1046A
> Layerscape Architecture processor:
> 
> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fconclusive.tech%2fproducts%2fwhle%2dls1%2dsbc%2f&umid=54a8c79f-0cf2-4f48-aefc-d35c76864d4f&auth=d807158c60b7d2502abde8a2fc01f40662980862-cf53e7a0650a67cfac22b122ae0b3fcf9018c729
>  
> +config TARGET_WHLE_LS1088A
> + bool "Support Conclusive WHLE-LS1088A"
> + select ARCH_LS1088A
> + select ARM64
> + select ARMV8_MULTIENTRY
> + select ARCH_SUPPORT_TFABOOT
> + select BOARD_EARLY_INIT_F
> + select BOARD_LATE_INIT
> + select GPIO_EXTRA_HEADER
> + select DM_SPI_FLASH if DM_SPI
> + imply SCSI
> + help
> +   Support for Conclusive WHLE-LS1088A platform.
> +   The WHLE-LS1088A is a high-performance Single Board Computer with
> +   extensive connectivity features that supports the QorIQ LS1088A
> +   Layerscape Architecture processor:
> +   
> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fconclusive.tech%2fproducts%2fwhle%2dls1%2dsbc%2f&umid=54a8c79f-0cf2-4f48-aefc-d35c76864d4f&auth=d807158c60b7d2502abde8a2fc01f40662980862-cf53e7a0650a67cfac22b122ae0b3fcf9018c729
> +
>  config TARGET_TEN64
>   bool "Support ten64"
>   select ARCH_LS1088A
> @@ -2318,6 +2336,7 @@ source "board/broadcom/bcmns/Kconfig"
>  source "board/broadcom/bcmns3/Kconfig"
>  source "board/cavium/thunderx/Kconfig"
>  source "board/conclusive/whle-ls1046a/Kconfig"
> +source "board/conclusive/whle-ls1088a/Kconfig"
>  source "board/eets/pdu001/Kconfig"
>  source "board/emulation/qemu-arm/Kconfig"
>  source "board/freescale/ls2080aqds/Kconfig"
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 8dcbf29df363..b0782b4f29bc 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -591,6 +591,7 @@ dtb-$(CONFIG_FSL_LSCH3) += fsl-ls2080a-qds.dtb \
>   fsl-ls1088a-qds.dtb \
>   fsl-ls1088a-qds-21-x.dtb \
>   fsl-ls1088a-qds-29-x.dtb \
> + fsl-ls1088a-whle.dtb \
>   fsl-ls1028a-rdb.dtb \
>   fsl-ls1028a-qds-duart.dtb \
>   fsl-ls1028a-qds-lpuart.dtb \
> diff --git a/arch/arm/dts/fsl-ls1088a-whle-u-boot.dtsi 
> b/arch/arm/dts/fsl-ls1088a-whle-u-boot.dtsi
> new file mode 100644
> index ..bbe93a1d6e4f
> --- /dev/null
> +++ b/arch/arm/dts/fsl-ls1088a-whle-u-boot.dtsi
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR X11
> +/*
> + * Copyright 2020-2023 Conclusive Engineering Sp. z o. o.
> + */
> +
> +#include 
> +
> +#include "fsl-ls1088a-u-boot.dtsi"
> diff --git a/arch/arm/dts/fsl-ls1088a-whle.dts 
> b/arch/arm/dts/fsl-ls1088a-whle.dts
> new file mode 100644
> index ..76ef1c748059
> --- /dev/null
> +++ b/arch/arm/dts/fsl-ls1088a-whle.dts
> @@ -0,0 +1,235 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR

Re: [PATCH v3 2/3] board: Add support for Conclusive WHLE-LS1046A

[Sean Anderson]

On 11/28/23 05:34, Artur Rojek wrote:
> Introduce support for Conclusive WHLE-LS1046A Single Board Computer.
> 
> Co-developed-by: Jakub Klama 
> Signed-off-by: Jakub Klama 
> Signed-off-by: Artur Rojek 
> ---
> 
> v3: no change
> 
> v2: - drop non-DM_ETH case
> - clean-up defines in configs/whle_ls1046a.h: remove unneeded ones,
>   move others to appropriate files in board directory
> - move environment variables to whle-ls1046a.env
> - move away from distro_bootcmd and use BOOTSTD 
> - fix i2c-mux node parent and ext_i2c address in Device Tree
> - style changes to eth.c
> - fix CONFIG_MTDPARTS_DEFAULT value in defconfigs
> 
>  arch/arm/Kconfig  |  19 ++
>  arch/arm/dts/Makefile |   1 +
>  arch/arm/dts/fsl-ls1046a-whle.dts | 208 +
>  board/conclusive/whle-ls1046a/Kconfig |  15 ++
>  board/conclusive/whle-ls1046a/MAINTAINERS |   9 +
>  board/conclusive/whle-ls1046a/Makefile|   7 +
>  board/conclusive/whle-ls1046a/ddr.c   |  21 ++
>  board/conclusive/whle-ls1046a/eth.c   |  65 ++
>  board/conclusive/whle-ls1046a/whle-ls1046a.c  | 215 ++
>  .../conclusive/whle-ls1046a/whle-ls1046a.env  |  13 ++
>  configs/whle_ls1046a_emmc_defconfig   |  83 +++
>  configs/whle_ls1046a_qspi_defconfig   |  84 +++
>  include/configs/whle_ls1046a.h|  47 
>  13 files changed, 787 insertions(+)
>  create mode 100644 arch/arm/dts/fsl-ls1046a-whle.dts
>  create mode 100644 board/conclusive/whle-ls1046a/Kconfig
>  create mode 100644 board/conclusive/whle-ls1046a/MAINTAINERS
>  create mode 100644 board/conclusive/whle-ls1046a/Makefile
>  create mode 100644 board/conclusive/whle-ls1046a/ddr.c
>  create mode 100644 board/conclusive/whle-ls1046a/eth.c
>  create mode 100644 board/conclusive/whle-ls1046a/whle-ls1046a.c
>  create mode 100644 board/conclusive/whle-ls1046a/whle-ls1046a.env
>  create mode 100644 configs/whle_ls1046a_emmc_defconfig
>  create mode 100644 configs/whle_ls1046a_qspi_defconfig
>  create mode 100644 include/configs/whle_ls1046a.h
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index d812685c9842..609571e6e421 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1851,6 +1851,24 @@ config TARGET_SL28
>   help
> Support for Kontron SMARC-sAL28 board.
>  
> +config TARGET_WHLE_LS1046A
> + bool "Support Conclusive WHLE-LS1046A"
> + select ARCH_LS1046A
> + select ARM64
> + select ARMV8_MULTIENTRY
> + select ARCH_SUPPORT_TFABOOT
> + select BOARD_EARLY_INIT_F
> + select BOARD_LATE_INIT
> + select GPIO_EXTRA_HEADER
> + select DM_SPI_FLASH if DM_SPI
> + imply SCSI
> + help
> +   Support for Conclusive WHLE-LS1046A platform.
> +   The WHLE-LS1046A is a high-performance Single Board Computer with
> +   extensive connectivity features that supports the QorIQ LS1046A
> +   Layerscape Architecture processor:
> +   
> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fconclusive.tech%2fproducts%2fwhle%2dls1%2dsbc%2f&umid=bfe70e04-e3cb-4832-a510-cd7686941d3c&auth=d807158c60b7d2502abde8a2fc01f40662980862-65691d364ff53ae48e02cdbc52323838b4503b62
> +
>  config TARGET_TEN64
>   bool "Support ten64"
>   select ARCH_LS1088A
> @@ -2299,6 +2317,7 @@ source "board/cortina/presidio-asic/Kconfig"
>  source "board/broadcom/bcmns/Kconfig"
>  source "board/broadcom/bcmns3/Kconfig"
>  source "board/cavium/thunderx/Kconfig"
> +source "board/conclusive/whle-ls1046a/Kconfig"
>  source "board/eets/pdu001/Kconfig"
>  source "board/emulation/qemu-arm/Kconfig"
>  source "board/freescale/ls2080aqds/Kconfig"
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 1be08c5fdc2e..8dcbf29df363 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -615,6 +615,7 @@ dtb-$(CONFIG_FSL_LSCH2) += fsl-ls1043a-qds-duart.dtb \
>   fsl-ls1046a-qds-lpuart.dtb \
>   fsl-ls1046a-rdb.dtb \
>   fsl-ls1046a-frwy.dtb \
> + fsl-ls1046a-whle.dtb \
>   fsl-ls1012a-qds.dtb \
>   fsl-ls1012a-rdb.dtb \
>   fsl-ls1012a-2g5rdb.dtb \
> diff --git a/arch/arm/dts/fsl-ls1046a-whle.dts 
> b/arch/arm/dts/fsl-ls1046a-whle.dts
> new file mode 100644
> index ..1aed3e8c4701
> --- /dev/null
> +++ b/arch/arm/dts/fsl-ls1046a-whle.dts
> @@ -0,0 +1,208 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR X11
> +/*
> + * Copyright 2020-2023 Conclusive Engineering Sp. z o. o.
> + */
> +
> +#include 
> +
> +/dts-v1/;
> +#include "fsl-ls1046a.dtsi"
> +
> +/ {
> + model = "Conclusive WHLE-LS1046A";
> + compatible = "conclusive,whle-ls1046a", "fsl,ls1046a";
> +
> + chosen {
> + stdout-path = &duart0;
> + };
> +
> + aliases {
> + spi0 = &qspi;
> + };
> +};
> +
> +&soc {
> + pcie@340 {
> + status = "okay";
> + };
> +};
> +
> +&qspi {
> +

[PATCH] lib: sparse: Fix error checking for write_sparse_chunk_raw

[Sean Anderson]

The return value of write_sparse_chunk_raw is unsigned, so the existing
check has no effect. Use IS_ERR_VALUE to detect error instead, which is
what write_sparse_chunk_raw does itself.

Fixes: 62649165cb0 ("lib: sparse: Make CHUNK_TYPE_RAW buffer aligned")
Reported-by: Dan Carpenter 
Link: 
https://lore.kernel.org/u-boot/1b323ec3-59b0-490b-a2f0-fd961dafcf49@moroto.mountain/
Signed-off-by: Sean Anderson 
---

 lib/image-sparse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/image-sparse.c b/lib/image-sparse.c
index f8289064692..09225692e9b 100644
--- a/lib/image-sparse.c
+++ b/lib/image-sparse.c
@@ -211,7 +211,7 @@ int write_sparse_image(struct sparse_storage *info,
 
blks = write_sparse_chunk_raw(info, blk, blkcnt,
  data, response);
-   if (blks < 0)
+   if (IS_ERR_VALUE(blks))
return -1;
 
blk += blks;
-- 
2.35.1.1320.gc452695387.dirty

[GIT PULL] Clock changes for v2024.04

[Sean Anderson]

The following changes since commit 6faba41927bdc8973b59678649ef83c564cc421e:

  Prepare v2024.04-rc1 (2024-01-29 20:53:19 -0500)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-clk.git tags/clk-2024.04-rc2

for you to fetch changes up to a8dc4965f09d28a59c156437673ddb66860c847e:

  clk: clk-gpio: add actual gated clock (2024-01-29 22:35:34 -0500)


Clock changes for v2024.04

This pull has the usual fixes and new (clock-adjacent) drivers. It also has some
cleanups for the clock API; in particular removing the unused rfree callback.

CI: https://source.denx.de/u-boot/custodians/u-boot-clk/-/pipelines/19486


Julien Masson (1):
  clk: fix clk_get_rate() always return ulong

Neil Armstrong (1):
  clk: meson: add Hardware Clock measure driver

Sean Anderson (3):
  clk: Remove rfree
  treewide: Remove clk_free
  clk: Document clk_ops return codes and behavior

Svyatoslav Ryhel (1):
  clk: clk-gpio: add actual gated clock

 arch/arm/mach-rockchip/rk3288/rk3288.c|   2 -
 arch/arm/mach-socfpga/clock_manager_agilex.c  |   2 -
 arch/arm/mach-socfpga/clock_manager_arria10.c |   7 +-
 arch/arm/mach-socfpga/clock_manager_n5x.c |   2 -
 arch/arm/mach-zynq/clk.c  |   2 -
 arch/mips/mach-pic32/cpu.c|   7 +-
 arch/sandbox/include/asm/clk.h|   8 --
 board/microchip/pic32mzda/pic32mzda.c |   2 -
 board/sipeed/maix/maix.c  |   1 -
 board/synopsys/hsdk/clk-lib.c |   2 -
 drivers/clk/aspeed/clk_ast2600.c  |   2 -
 drivers/clk/at91/compat.c |  14 +-
 drivers/clk/clk-gpio.c|  38 +-
 drivers/clk/clk-uclass.c  |  47 +--
 drivers/clk/clk-xlnx-clock-wizard.c   |   1 -
 drivers/clk/clk_sandbox.c |  12 --
 drivers/clk/clk_sandbox_test.c|  12 --
 drivers/clk/clk_versaclock.c  |  12 +-
 drivers/clk/clk_zynq.c|   2 -
 drivers/clk/clk_zynqmp.c  |   2 -
 drivers/clk/imx/clk-imx8.c|   2 -
 drivers/clk/meson/Kconfig |  10 ++
 drivers/clk/meson/Makefile|   1 +
 drivers/clk/meson/clk-measure.c   | 634 
++
 drivers/clk/mvebu/armada-37xx-periph.c|   2 -
 drivers/cpu/riscv_cpu.c   |   2 -
 drivers/dma/bcm6348-iudma.c   |   2 -
 drivers/gpio/at91_gpio.c  |   2 -
 drivers/gpio/atmel_pio4.c |   2 -
 drivers/gpio/gpio-rcar.c  |   1 -
 drivers/hwspinlock/stm32_hwspinlock.c |   6 +-
 drivers/i2c/at91_i2c.c|   2 -
 drivers/i2c/designware_i2c.c  |   2 -
 drivers/i2c/i2c-microchip.c   |   2 -
 drivers/i2c/npcm_i2c.c|   1 -
 drivers/i2c/ocores_i2c.c  |   2 -
 drivers/i2c/stm32f7_i2c.c |   4 +-
 drivers/mailbox/stm32-ipcc.c  |   7 +-
 drivers/misc/ls2_sfp.c|   1 -
 drivers/mmc/arm_pl180_mmci.c  |   1 -
 drivers/mmc/aspeed_sdhci.c|   4 +-
 drivers/mmc/atmel_sdhci.c |   2 -
 drivers/mmc/gen_atmel_mci.c   |  19 +--
 drivers/mmc/msm_sdhci.c   |   1 -
 drivers/mmc/pic32_sdhci.c |   1 -
 drivers/mmc/renesas-sdhi.c|  21 +--
 drivers/mmc/snps_dw_mmc.c |   8 +-
 drivers/mmc/socfpga_dw_mmc.c  |   1 -
 drivers/mmc/stm32_sdmmc2.c|   4 +-
 drivers/mmc/uniphier-sd.c |   1 -
 drivers/mtd/nand/raw/atmel/nand-controller.c  |   4 +-
 drivers/mtd/renesas_rpc_hf.c  |   1 -
 drivers/net/bcm6348-eth.c |   2 -
 drivers/net/bcm6368-eth.c |   2 -
 drivers/net/designware.c  |   1 -
 drivers/net/dwc_eth_qos.c |  43 +-
 drivers/net/dwc_eth_qos_imx.c |  21 +--
 drivers/net/dwc_eth_qos_qcom.c|   1 -
 drivers/net/dwc_eth_qos_rockchip.c|   6 +-
 drivers/net/sni_ave.c |   5 +-
 drivers/net/ti/am65-cpsw-nuss.c   |   1 -
 drivers/phy/bcm6318-usbh-phy.c|   2 -
 drivers/phy/bcm6348-usbh-phy.c|   2 -
 drivers/phy/bcm6368-usbh-phy.c|   4 -
 drivers/phy/meson-axg-mipi-dphy.c |   1 -
 drivers/phy/meson-g12a-usb3-pcie.c|   1 -
 drivers/phy/meson-gxl-usb2.c  |   1 -
 drivers/phy/phy-rcar-gen2.c   |   1

Re: [PATCH 0/3] clk: Remove clk_free and better document clk_ops

[Sean Anderson]

On Sat, 16 Dec 2023 14:38:40 -0500, Sean Anderson wrote:
> This series contains two unrelated changes. They are included together because
> they touch the same files and would otherwise conflict. The first change is to
> completely remove clk_free. This is because the op it calls (rfree) is
> completely unused. I believe the original intent of this op was to allow clock
> drivers to free resources allocated in request. However, no clock drivers do
> this, so we can remove the function and all its callers.
> 
> [...]

Applied, thanks!

[1/3] clk: Remove rfree
  https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/82719d3f409f
[2/3] treewide: Remove clk_free
  https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/c9309f40a683
[3/3] clk: Document clk_ops return codes and behavior
  https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/0bfbb830554f

Best regards,
-- 
Sean Anderson 

Re: [PATCH v2] clk: fix clk_get_rate() always return ulong

[Sean Anderson]

On Fri, 15 Dec 2023 15:09:43 +0100, Julien Masson wrote:
> When we call clk_get_rate(), we expect to get clock rate value as
> ulong.
> In that case we should not use log_ret() macro since it use internally
> an int.
> Otherwise we may return an invalid/truncated clock rate value.
> 
> 
> [...]

Applied, thanks!

[1/1] clk: fix clk_get_rate() always return ulong
  https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/b500447ad6ae

Best regards,
-- 
Sean Anderson 

Re: [PATCH v3] clk: meson: add Hardware Clock measure driver

[Sean Anderson]

On Mon, 18 Dec 2023 10:47:55 +0100, Neil Armstrong wrote:
> Amlogic SoCs embeds an hardware clock measure block, port it
> from Linux and implement it as a UCLK_CLK with only the dump
> op and fail-only xlate.
> 
> Based on the Linux driver introduced in [1].
> 
> [1] commit 2b45ebef39a2 ("soc: amlogic: Add Meson Clock Measure driver").
> 
> [...]

Applied, thanks!

[1/1] clk: meson: add Hardware Clock measure driver
  https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/2da1331d20b3

Best regards,
-- 
Sean Anderson 

Re: [PATCH v3 0/1] Fix clk-gpio driver

[Sean Anderson]

On Wed, 10 Jan 2024 18:09:55 +0200, Svyatoslav Ryhel wrote:
> Existing gpio-gate-clock driver acts like a simple GPIO switch without any
> effect on gated clock. Add actual clock actions into enable/disable ops and
> implement get_rate op by passing gated clock if it is enabled.
> 
> Testing current driver implementation shows that it is not fully capable
> and lacks gated clock manipulations.
> 
> [...]

Applied, thanks!

[1/1] clk: clk-gpio: add actual gated clock
  https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/a8dc4965f09d

Best regards,
-- 
Sean Anderson 

Re: [PATCH v3 1/1] clk: clk-gpio: add actual gated clock

[Sean Anderson]

On 1/10/24 11:09, Svyatoslav Ryhel wrote:

Existing gpio-gate-clock driver acts like a simple GPIO switch without any
effect on gated clock. Add actual clock actions into enable/disable ops and
implement get_rate op by passing gated clock if it is enabled.

Signed-off-by: Svyatoslav Ryhel 
---
  drivers/clk/clk-gpio.c | 40 ++--
  1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk-gpio.c b/drivers/clk/clk-gpio.c
index 26d795b978..d2dd8070fa 100644
--- a/drivers/clk/clk-gpio.c
+++ b/drivers/clk/clk-gpio.c
@@ -3,19 +3,23 @@
   * Copyright (C) 2023 Marek Vasut 
   */
  
-#include 

-#include 
-#include 
+#include 
  #include 
+#include 


this is out of order, but I will fix it when applying


+#include 
+
+#include 
  
  struct clk_gpio_priv {

-   struct gpio_descenable;
+   struct gpio_descenable; /* GPIO, controlling the gate */
+   struct clk  *clk;   /* Gated clock */
  };
  
  static int clk_gpio_enable(struct clk *clk)

  {
struct clk_gpio_priv *priv = dev_get_priv(clk->dev);
  
+	clk_enable(priv->clk);

dm_gpio_set_value(&priv->enable, 1);
  
  	return 0;

@@ -26,21 +30,45 @@ static int clk_gpio_disable(struct clk *clk)
struct clk_gpio_priv *priv = dev_get_priv(clk->dev);
  
  	dm_gpio_set_value(&priv->enable, 0);

+   clk_disable(priv->clk);
  
  	return 0;

  }
  
+static ulong clk_gpio_get_rate(struct clk *clk)

+{
+   struct clk_gpio_priv *priv = dev_get_priv(clk->dev);
+
+   return clk_get_rate(priv->clk);
+}
+
  const struct clk_ops clk_gpio_ops = {
.enable = clk_gpio_enable,
.disable= clk_gpio_disable,
+   .get_rate   = clk_gpio_get_rate,
  };
  
  static int clk_gpio_probe(struct udevice *dev)

  {
struct clk_gpio_priv *priv = dev_get_priv(dev);
+   int ret;
  
-	return gpio_request_by_name(dev, "enable-gpios", 0,

-   &priv->enable, GPIOD_IS_OUT);
+   priv->clk = devm_clk_get(dev, NULL);
+   if (IS_ERR(priv->clk)) {
+   log_debug("%s: Could not get gated clock: %ld\n",
+ __func__, PTR_ERR(priv->clk));
+   return PTR_ERR(priv->clk);
+   }
+
+   ret = gpio_request_by_name(dev, "enable-gpios", 0,
+  &priv->enable, GPIOD_IS_OUT);
+   if (ret) {
+   log_debug("%s: Could not decode enable-gpios (%d)\n",
+ __func__, ret);
+   return ret;
+   }
+
+   return 0;
  }
  
  /*


Reviewed-by: Sean Anderson