Re: [PATCH] pci: Fix device_find_first_child() return value handling
Hello, On Wed, Jul 26, 2023 at 06:49:57PM -0600, Simon Glass wrote: > Hi Marek, > > On Mon, 17 Jul 2023 at 11:03, Marek Vasut wrote: > > > > On 7/17/23 09:42, Michal Suchánek wrote: ... > > > More generally, what is the overall vision for these functions returning > > > always zero? > > > > > > Should the return value be kept in case the underlying implementation > > > changes and errors can happen in the future, and consequently checked? > > > > > > Should the return value be removed when meaningless making these > > > useless assignments and checks an error? > > > > > > I already elimimnated a return value where using it lead to incorrect > > > behavior but here using it or not is equally correct with the current > > > implementation. > > > > Probably a question for Simon, really. Personally I would be tempted to > > switch the function to return void. > > So long as the function has its meaning documented, I think it is OK. > As a separate patch, I am OK with changing > device_find_first/next_child() to void, or alternatively having them > return 0 on success and -ENODEV if nothing was found. I think when there is one error condition having two ways to report it is one too many. Thanks Michal
[PATCH 1/2] drivers: video: tidss: tidss_drv: Change remove method
Change remove method of DSS video driver to disable video port instead
of performing a soft reset, as soft reset takes longer duration. Video
port is disabled by setting enable bit of video port to 0.
Signed-off-by: Nikhil M Jain
---
drivers/video/tidss/tidss_drv.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/video/tidss/tidss_drv.c b/drivers/video/tidss/tidss_drv.c
index 078e3e82e3..623bf4cf31 100644
--- a/drivers/video/tidss/tidss_drv.c
+++ b/drivers/video/tidss/tidss_drv.c
@@ -901,19 +901,9 @@ static int tidss_drv_probe(struct udevice *dev)
static int tidss_drv_remove(struct udevice *dev)
{
- u32 val;
- int ret;
struct tidss_drv_priv *priv = dev_get_priv(dev);
- priv->base_common = dev_remap_addr_index(dev, 0);
- REG_FLD_MOD(priv, DSS_SYSCONFIG, 1, 1, 1);
- /* Wait for reset to complete */
- ret = readl_poll_timeout(priv->base_common + DSS_SYSSTATUS,
-val, val & 1, 5000);
- if (ret) {
- dev_warn(priv->dev, "failed to reset priv\n");
- return ret;
- }
+ VP_REG_FLD_MOD(priv, 0, DSS_VP_CONTROL, 0, 0, 0);
return 0;
}
--
2.34.1
[PATCH 2/2] drivers: video: tidss: tidss_drv: Use kconfig VIDEO_REMOVE to remove video
Perform removal of DSS if kconfigs VIDEO_REMOVE or SPL_VIDEO_REMOVE is
set by user. Otherwise if above Kconfigs are not selected, it is assumed
that user wants splash screen to be displayed until linux kernel boots
up. In such scenario, leave the power domain of DSS as "on" so that
splash screen stays intact until kernel boots up.
Signed-off-by: Nikhil M Jain
---
drivers/video/tidss/tidss_drv.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/video/tidss/tidss_drv.c b/drivers/video/tidss/tidss_drv.c
index 623bf4cf31..e285f255d7 100644
--- a/drivers/video/tidss/tidss_drv.c
+++ b/drivers/video/tidss/tidss_drv.c
@@ -901,9 +901,11 @@ static int tidss_drv_probe(struct udevice *dev)
static int tidss_drv_remove(struct udevice *dev)
{
- struct tidss_drv_priv *priv = dev_get_priv(dev);
+ if (CONFIG_IS_ENABLED(VIDEO_REMOVE)) {
+ struct tidss_drv_priv *priv = dev_get_priv(dev);
- VP_REG_FLD_MOD(priv, 0, DSS_VP_CONTROL, 0, 0, 0);
+ VP_REG_FLD_MOD(priv, 0, DSS_VP_CONTROL, 0, 0, 0);
+ }
return 0;
}
@@ -929,5 +931,9 @@ U_BOOT_DRIVER(tidss_drv) = {
.probe = tidss_drv_probe,
.remove = tidss_drv_remove,
.priv_auto = sizeof(struct tidss_drv_priv),
+#if CONFIG_IS_ENABLED(VIDEO_REMOVE)
.flags = DM_FLAG_OS_PREPARE,
+#else
+ .flags = DM_FLAG_OS_PREPARE | DM_FLAG_LEAVE_PD_ON,
+#endif
};
--
2.34.1
[PATCH 0/2] Update remove method for DSS driver
This patch series aims at updating the remove method for DSS video driver. Nikhil M Jain (2): drivers: video: tidss: tidss_drv: Change remove method drivers: video: tidss: tidss_drv: Use kconfig VIDEO_REMOVE to remove video drivers/video/tidss/tidss_drv.c | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) -- 2.34.1
Re: [PATCH v4 1/9] spl: Add generic spl_load function
On 7/24/23 19:12, Sean Anderson wrote:
Implementers of SPL_LOAD_IMAGE_METHOD have to correctly determine what
type of image is being loaded and then call the appropriate image load
function correctly. This is tricky, because some image load functions
expect the whole image to already be loaded (CONFIG_SPL_LOAD_FIT_FULL),
some will load the image automatically using spl_load_info.read()
(CONFIG_SPL_LOAD_FIT/CONFIG_SPL_LOAD_IMX_CONTAINER), and some just parse
the header and expect the caller to do the actual loading afterwards
(legacy/raw images). Load methods often only support a subset of the
above methods, meaning that not all image types can be used with all
load methods. Further, the code to invoke these functions is
duplicated between different load functions.
To address this problem, this commit introduces a "spl_load" function.
It aims to handle image detection and correct invocation of each of the
parse/load functions. spl_simple_read is a wrapper around
spl_load_info.read with get_aligned_image* functions inlined for size
purposes. Additionally, we assume that bl_len is a power of 2 so we can
do bitshifts instead of divisions (which is smaller and faster).
Signed-off-by: Sean Anderson
Reviewed-by: Stefan Roese
---
Changes in v4:
- Fix format specifiers in debug prints
- Reword/fix some of the doc comments for spl_load
Changes in v3:
- Fix using ffs instead of fls
- Fix using not initializing bl_len when info->filename was NULL
Changes in v2:
- Use reverse-xmas-tree style for locals in spl_simple_read. This is not
complete, since overhead depends on bl_mask.
common/spl/spl.c | 68
include/spl.h| 29 -
2 files changed, 96 insertions(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c
index f09bb977814..3ef064009e8 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -450,6 +450,74 @@ int spl_parse_image_header(struct spl_image_info
*spl_image,
return 0;
}
+static int spl_simple_read(struct spl_load_info *info, void *buf, size_t size,
+ size_t offset)
+{
+ size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : info->bl_len;
+ size_t bl_mask = bl_len - 1;
+ size_t overhead = offset & bl_mask;
+ size_t bl_shift = fls(bl_mask);
+ int ret;
+
+ debug("%s: buf=%p size=%lx offset=%lx\n", __func__, buf, (long)size,
+ (long)offset);
+ debug("%s: bl_len=%lx bl_mask=%lx bl_shift=%lx\n", __func__,
(long)bl_len,
+ (long)bl_mask, (long)bl_shift);
+
+ buf -= overhead;
+ size = (size + overhead + bl_mask) >> bl_shift;
+ offset = offset >> bl_shift;
+
+ debug("info->read(info, %lx, %lx, %p)\n", (ulong)offset, (ulong)size,
+ buf);
+ ret = info->read(info, offset, size, buf);
+ return ret == size ? 0 : -EIO;
+}
+
+int spl_load(struct spl_image_info *spl_image,
+const struct spl_boot_device *bootdev, struct spl_load_info *info,
+struct legacy_img_hdr *header, size_t size, size_t sector)
Hello Sean,
carving out common functionality is really a good path forward.
This function spl_load() receives a pointer to a read function in
info->read() and additionally the file header.
Why can't we move the reading of the header to spl_load() too?
Best regards
Heinrich
+{
+ int ret;
+ size_t offset = sector * info->bl_len;
+
+ if (image_get_magic(header) == FDT_MAGIC) {
+ if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL)) {
+ void *buf;
+
+ /*
+* In order to support verifying images in the FIT, we
+* need to load the whole FIT into memory. Try and
+* guess how much we need to load by using the total
+* size. This will fail for FITs with external data,
+* but there's not much we can do about that.
+*/
+ if (!size)
+ size = roundup(fdt_totalsize(header), 4);
+ buf = spl_get_load_buffer(0, size);
+ ret = spl_simple_read(info, buf, size, offset);
+ if (ret)
+ return ret;
+
+ return spl_parse_image_header(spl_image, bootdev, buf);
+ }
+
+ if (IS_ENABLED(CONFIG_SPL_LOAD_FIT))
+ return spl_load_simple_fit(spl_image, info, sector,
+ header);
+ }
+
+ if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER))
+ return spl_load_imx_container(spl_image, info, sector);
+
+ ret = spl_parse_image_header(spl_image, bootdev, header);
+ if (ret)
+ return ret;
+
+ return spl_simple_read(info, (void *)spl_image->load_addr,
+
Re: [PATCH 4/5] mmc: Use regulator_set_enable_if_allowed
чт, 20 лип. 2023 р. о 00:21 Jonas Karlman пише:
>
> With the commit 4fcba5d556b4 ("regulator: implement basic reference
> counter") the return value of regulator_set_enable may be EALREADY or
> EBUSY for fixed/gpio regulators.
>
> Change to use the more relaxed regulator_set_enable_if_allowed to
> continue if regulator already was enabled or disabled.
>
> Signed-off-by: Jonas Karlman
> ---
> drivers/mmc/mmc.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
Tested-by: Svyatoslav Ryhel # P895 Tegra 3;
Tegratab Tegra 4
Re: [PATCH 2/6] arm_ffa: introduce armffa command
On 3/29/22 17:16, abdellatif.elkhl...@arm.com wrote:
From: Abdellatif El Khlifi
Provide armffa command showcasing the use of the FF-A driver
The armffa command allows to query secure partitions data from
the secure world and exchanging messages with the partitions.
Signed-off-by: Abdellatif El Khlifi
Cc: Tom Rini
---
MAINTAINERS | 1 +
cmd/Kconfig | 10 ++
cmd/Makefile| 2 +
cmd/armffa.c| 266
drivers/arm-ffa/Kconfig | 1 +
We want to have all commands to be documented in /doc/usage/cmd/.
Could you, please, provide the missing /doc/usage/cmd/armffa.rst file.
Best regards
Heinrich
5 files changed, 280 insertions(+)
create mode 100644 cmd/armffa.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c5b608eb60..50ccd6a7ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -235,6 +235,7 @@ F: include/configs/turris_*.h
ARM FF-A
M:Abdellatif El Khlifi
S:Maintained
+F: cmd/armffa.c
F:drivers/arm-ffa/
F:include/arm_ffa.h
F:include/arm_ffa_helper.h
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 79219bcb74..de5bea1404 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -813,6 +813,16 @@ endmenu
menu "Device access commands"
+config CMD_ARMFFA
+ bool "Arm FF-A test command"
+ depends on ARM_FFA_TRANSPORT
+ help
+ Provides a test command for the Arm FF-A driver
+ supported options:
+ - Listing the partition(s) info
+ - Sending a data pattern to the specified partition
+ - Displaying the arm_ffa device info
+
config CMD_ARMFLASH
#depends on FLASH_CFI_DRIVER
bool "armflash"
diff --git a/cmd/Makefile b/cmd/Makefile
index ede634d731..2905ce63cf 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -12,6 +12,8 @@ obj-y += panic.o
obj-y += version.o
# command
+
+obj-$(CONFIG_CMD_ARMFFA) += armffa.o
obj-$(CONFIG_CMD_ACPI) += acpi.o
obj-$(CONFIG_CMD_ADDRMAP) += addrmap.o
obj-$(CONFIG_CMD_AES) += aes.o
diff --git a/cmd/armffa.c b/cmd/armffa.c
new file mode 100644
index 00..fcfc3a06bd
--- /dev/null
+++ b/cmd/armffa.c
@@ -0,0 +1,266 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2022 ARM Limited
+ * Abdellatif El Khlifi
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * do_ffa_get_singular_partition_info - implementation of the getpart
subcommand
+ * @cmdtp: Command Table
+ * @flag: flags
+ * @argc: number of arguments
+ * @argv: arguments
+ *
+ * This function queries the secure partition information which the UUID is
provided
+ * as an argument. The function uses the arm_ffa driver helper function
+ * to retrieve the data.
+ * The input UUID string is expected to be in big endian format.
+ *
+ * Return:
+ *
+ * CMD_RET_SUCCESS: on success, otherwise failure
+ */
+static int do_ffa_get_singular_partition_info(struct cmd_tbl *cmdtp, int flag,
int argc,
+ char *const argv[])
+{
+ struct ffa_interface_data func_data = {0};
+ u32 count = 0;
+ int ret;
+ union ffa_partition_uuid service_uuid = {0};
+ struct ffa_partition_info *parts_info;
+ u32 info_idx;
+
+ if (argc != 1)
+ return -EINVAL;
+
+ if (ffa_uuid_str_to_bin(argv[0], (unsigned char *)&service_uuid)) {
+ ffa_err("Invalid UUID");
+ return -EINVAL;
+ }
+
+ /*
+* get from the driver the count of the SPs matching the UUID
+*/
+ func_data.data0_size = sizeof(service_uuid);
+ func_data.data0 = &service_uuid;
+ func_data.data1_size = sizeof(count);
+ func_data.data1 = &count;
+
+ ret = ffa_helper_get_partitions_info(&func_data);
+ if (ret != FFA_ERR_STAT_SUCCESS) {
+ ffa_err("Failure in querying partitions count (error code:
%d)", ret);
+ return ret;
+ }
+
+ if (!count) {
+ ffa_info("No secure partition found");
+ return ret;
+ }
+
+ /*
+* pre-allocate a buffer to be filled by the driver
+* with ffa_partition_info structs
+*/
+
+ parts_info = calloc(count, sizeof(struct ffa_partition_info));
+ if (!parts_info)
+ return -EINVAL;
+
+ ffa_info("Pre-allocating %d partition(s) info structures", count);
+
+ func_data.data1_size = count * sizeof(struct ffa_partition_info);
+ func_data.data1 = parts_info;
+
+ /*
+* ask the driver to fill the buffer with the SPs info
+*/
+ ret = ffa_helper_get_partitions_info(&func_data);
+ if (ret != FFA_ERR_STAT_SUCCESS) {
+ ffa_err("Failure in querying partition(s) info (error code:
%d)", ret);
+ free(parts_info);
+ return ret;
+ }
+
+ /*
+* SPs found , show
Re: [PATCH 0/4] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl
On 7/26/23 6:22 PM, Nishanth Menon wrote: > On 14:44-20230726, Ravi Gunasekaran wrote: > [...] >> If it is the former, then would a duplicate mdio node help keep the changes >> to minimum? > > That is worse hack. How does it make sense to have two mdio nodes to > represent the same hardware block? we are trying to get closer to kernel > dts support not farther away from it. > I know it's not a clean hack. In k3-am625-sk-u-boot.dtsi, as a hack, the mdio pinctrl gets added to cpsw node. So was wondering if a duplicate mdio pinctrl node could be added in the same file. Just wanted to know if we could skip the changes in CPSW driver posted by Maxime. But Maxime's approach is much cleaner. We can just sync up kernel dts and not keeping adding the hack to every K3 SoC's DT. -- Regards, Ravi
Re: [PATCH 0/4] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl
Maxime, On 7/26/23 6:19 PM, Maxime Ripard wrote: > Hi Ravi, > > On Wed, Jul 26, 2023 at 02:44:00PM +0530, Ravi Gunasekaran wrote: >> On 7/20/23 7:42 PM, Maxime Ripard wrote: >>> Hi >>> >>> Sorry, I didn't receive Roger mail so I'll reply here >>> >>> On Thu, Jul 20, 2023 at 09:00:19AM -0500, Nishanth Menon wrote: On 16:56-20230720, Roger Quadros wrote: > Hi, > > On 20/07/2023 16:33, Ravi Gunasekaran wrote: >> >> >> On 7/20/2023 3:25 PM, Maxime Ripard wrote: >>> Hi, >>> >>> This series is based on: >>> https://lore.kernel.org/all/20230713072019.3153871-1...@ti.com/ >>> >>> It fixes the issue of Linux booting from the DT embedded by U-boot. The >>> main issue there is that U-Boot doesn't handle the MDIO child node that >>> might have resources attached to it. >>> >>> Thus, any pinctrl configuration that could be attached to the MDIO >>> child node is effectively ignored. Unfortunately, starting with 6.5-rc1, >>> Linux does just that. > > I didn't get this part. Linux does not ignore pinctrl configuration > attached > to MDIO child node. What changed in 6.5-rc1? >>> >>> Linux doesn't ignore it, but Linux started putting pinctrl configuration >>> on the MDIO node, which is ignored by U-Boot. >>> >>> This was solved by duplicating the pinctrl configuration onto the MAC > > If I remember right, there is no driver model driver for MDIO in u-boot > and > adding the mdio pinctrl into CPSW DT node was a hack in u-boot. >>> >>> Yeah, but we now have in the U-Boot DT two nodes referencing the same >>> pinctrl configuration: the CPSW and the MDIO node. Linux then tries to >>> assign that configuration to both devices and it fails. >> >> This response might be late, as I know things have moved ahead after this >> discussion. But I just wanted to understand the root cause little bit more. >> Is the issue mainly because the same mdio pinctrl node(phandle) is >> referenced in >> two other nodes? Or is it because of same pins being updated in two different >> places in the kernel? >> >> If it is the former, then would a duplicate mdio node help keep the changes >> to minimum? > > Neither, actually :/ The issue is that, with the changes made by > Nishanth to bring the 6.5-rc1 DTS in, the same pinctrl node is > referenced in two separate nodes, and Linux sees that as conflicting > users of the same pins. So you mean to say, finally it boils down to the users of the same pins. Even if there are two nodes with the same pinctrl configuration, we would still hit the issue. > > One quick workaround would be to remove the MDIO pinctrl property, and > add it to the MAC pinctrl property. > > That's fairly dangerous if either get extended though, so it might not > be a proper solution long term. A proper solution would be to update the MDIO driver. I'm sorry that it doesn't follow the standard driver model. We have plans to fix it soon. > > I hope it's clearer > > Maxime -- Regards, Ravi
Re: [PATCH 5/9] board_f: Fix corruption of relocaddr
Hi Simon,
On 27/07/23 06:23, Simon Glass wrote:
Hi Devarsh,
On Wed, 26 Jul 2023 at 05:09, Devarsh Thakkar wrote:
Hi Simon,
On 26/07/23 02:58, Simon Glass wrote:
Hi Devarsh,
On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar wrote:
Hi Simon,
On 24/07/23 20:22, Simon Glass wrote:
When the video framebuffer comes from the bloblist, we should not change
relocaddr to this address, since it interfers with the normal memory
allocation.
This fixes a boot loop in qemu-x86_64
Signed-off-by: Simon Glass
Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to
u-boot")
---
common/board_f.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c
index 7d2c380e91e2..5c8646b22283 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -419,7 +419,6 @@ static int reserve_video(void)
if (!ho)
return log_msg_ret("blf", -ENOENT);
video_reserve_from_bloblist(ho);
- gd->relocaddr = ho->fb;
I think this change was done as relocaddr pointer was required to be updated
to move after frame-buffer reserved area to ensure that any further memory
reservations done using gd->relocaddr (for e.g. in reserve_trace/uboot/malloc)
don't overlap with frame-buffer reserved area passed from blob, so I think
removing this line may cause further memory reservations to overlap with
reserved framebuffer.
Could you please confirm?
SPL and U-Boot have different memory layouts. The current code is
interrupting U-Boot's careful building up of chunks of memory used for
different purposes.
But it is possible they could be using similar memory layout for some
components like framebuffer.
For e.g. in our case we are using same video_reserve func in A53 SPL too
and which allocates framebuffer from gd->relocaddr as seen here:
https://source.denx.de/u-boot/u-boot/-/blob/v2023.10-rc1/common/board_f.c?ref_type=tags#L427
Even if it is similar, the point is that U-Boot proper needs to do its
own allocation stuff.
Of course, if SPL sets up the video, it will provide the framebuffer
address, but only that. The other addresses need to be done using the
normal mechanism.
The video memory has already been allocated by SPL, so we don't need
to insert a new one here, as your code demonstrates.
Agreed.
But also we have no way of knowing if it is legal to relocate U-Boot
(and various other things) just below the frame buffer chosen by SPL.
Yes, so i suppose your case is that framebuffer address which is being passed
by SPL is totally disjoint and too far away from gd->relocaddr, for e.g. it is
at the start (or bottom of DDR) and doesn't interfere with gd->relocaddr in
any manner since relocaddr points to ramtop (i.e. near to end address of DDR).
In that case I agree it doesn't make sense to move relocaddr to ho->fb.
But for the scenario where gd->relocaddr and ho->fb are nearby there is every
possibility that gd->relocaddr may overlap with framebuffer, also the code in
reserve_trace, reserve_uboot doesn't have any intelligence or check that it is
overlapping with framebuffer area or not.
I think one thing that can probably be done here is to have a check that if
passed framebuffer area falls within current relocaddr region, then update the
relocaddr else don't touch relocaddr :
if (ho->fb <= gd->relocaddr - ho->size)
//It means framebuffer are is overlapping with current relocaddr so update
relocaddr
gd->relocaddr = ho->fb
else
//don't update gd->relocaddr since ho->fb is disjoint to gd->relocaddr
Could you please share your opinion on this and if above logic suffice your
case too ?
I don't think this line is needed at all, which is why this patch
removes it. What problem are you seeing?
Across SPL stage and U-boot we are keeping same memory layout and
ensuring that same memory regions are used, this way it doesn't
interfere in the way of u-boot while allocating memory regions for
various purposes. This allowed us to display splash screen without any
flicker across the stages.
Now if you remove the line gd->relocaddr = ho->fb, the frame buffer
region will be used for reserving memory for other purposes which
corrupts the frame buffer.
One solution which we are planning to implement is move the ram_top to a
lower address leaving out a region for video buffer and u-boot can do
the allocation from the new ram_top address without spl video handoff
interfering in the u-boot's allocation of memory.The region above the
ram_top can be used for video.
Present Scenario
+-+ram_top
| |
| page_table |
| |
| |
+-+
| |
| |
| |
| |
| |
| video frame buffer |
| |
| |
| |
| |
| |
| |
+--
[PATCH 5/5] configs: iot2050: Enabled keyed autoboot
From: Jan Kiszka Only accept SPACE to stop autobooting. This is safer to avoid accidental interruptions on unattended devices. Signed-off-by: Jan Kiszka --- configs/iot2050_defconfig | 4 1 file changed, 4 insertions(+) diff --git a/configs/iot2050_defconfig b/configs/iot2050_defconfig index b5a50d4fa4a..bcbaa92ee89 100644 --- a/configs/iot2050_defconfig +++ b/configs/iot2050_defconfig @@ -36,6 +36,10 @@ CONFIG_DISTRO_DEFAULTS=y CONFIG_BOOTSTAGE=y CONFIG_SHOW_BOOT_PROGRESS=y CONFIG_SPL_SHOW_BOOT_PROGRESS=y +CONFIG_AUTOBOOT_KEYED=y +CONFIG_AUTOBOOT_FLUSH_STDIN=y +CONFIG_AUTOBOOT_PROMPT="Hit SPACE to stop autoboot in %d seconds...\n" +CONFIG_AUTOBOOT_STOP_STR=" " CONFIG_BOOTCOMMAND="run start_watchdog; run distro_bootcmd" CONFIG_CONSOLE_MUX=y # CONFIG_DISPLAY_CPUINFO is not set -- 2.35.3
[PATCH 3/5] boards: siemens: iot2050: Unify PG1 and PG2/M.2 configurations again
From: Jan Kiszka
This avoids having to maintain to defconfigs that are 99% equivalent.
The approach is to use binman to generate two flash images,
flash-pg1.bin and flash-pg2.bin. With the help of a template dtsi, we
can avoid duplicating the common binman image definitions.
Suggested-by: Andrew Davis
Signed-off-by: Jan Kiszka
---
CC: Andrew Davis
---
arch/arm/dts/k3-am65-iot2050-boot-image.dtsi | 155 +++---
board/siemens/iot2050/Kconfig | 30 +---
board/siemens/iot2050/MAINTAINERS | 3 +-
board/siemens/iot2050/board.c | 25 ++-
...ot2050_pg1_defconfig => iot2050_defconfig} | 3 +-
configs/iot2050_pg2_defconfig | 151 -
doc/board/siemens/iot2050.rst | 27 ++-
tools/iot2050-sign-fw.sh | 6 +-
8 files changed, 138 insertions(+), 262 deletions(-)
rename configs/{iot2050_pg1_defconfig => iot2050_defconfig} (97%)
delete mode 100644 configs/iot2050_pg2_defconfig
diff --git a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
index 7bfa4eebb90..3ecb461b011 100644
--- a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
+++ b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Copyright (c) Siemens AG, 2020-2022
+ * Copyright (c) Siemens AG, 2020-2023
*
* Authors:
* Jan Kiszka
@@ -10,23 +10,23 @@
#include
/ {
- binman {
- filename = "flash.bin";
+ binman: binman {
+ multiple-images;
+ };
+};
+
+&binman {
+ common_part: template {
pad-byte = <0xff>;
size = <0x8c>;
allow-repack;
- blob-ext@0x00 {
+ blob-ext@0 {
offset = <0x00>;
-#ifdef CONFIG_TARGET_IOT2050_A53_PG1
- filename = "seboot_pg1.bin";
-#else
- filename = "seboot_pg2.bin";
-#endif
missing-msg = "iot2050-seboot";
};
- fit@0x18 {
+ fit@18 {
offset = <0x18>;
filename = "tispl.bin";
pad-byte = <0xff>;
@@ -104,9 +104,8 @@
};
};
- fit@0x38 {
+ fit@38 {
description = "U-Boot for IOT2050";
- fit,fdt-list = "of-list";
offset = <0x38>;
images {
u-boot {
@@ -134,36 +133,6 @@
};
};
-#ifdef CONFIG_TARGET_IOT2050_A53_PG2
- bkey-usb3-overlay {
- description = "M.2-bkey-usb3-overlay";
- type = "blob";
- load = <0x8210>;
- arch = "arm64";
- compression = "none";
- blob-ext {
- filename =
"k3-am6548-iot2050-advanced-m2-bkey-usb3-overlay.dtbo";
- };
- hash {
- algo = "sha256";
- };
- };
-
- bkey-ekey-pcie-overlay {
- description =
"M.2-bkey-ekey-pcie-overlay";
- type = "blob";
- load = <0x8211>;
- arch = "arm64";
- compression = "none";
- blob-ext {
- filename =
"k3-am6548-iot2050-advanced-m2-bkey-ekey-pcie-overlay.dtbo";
- };
- hash {
- algo = "sha256";
- };
- };
-#endif
-
#ifdef CONFIG_WDT_K3_RTI_FW_FILE
k3-rti-wdt-firmware {
type = "firmware";
@@ -182,20 +151,10 @@
};
configurations {
- default = "@config-DEFAULT-SEQ";
@config-SEQ {
description = "NAME";
firmware = "u-boot";
fdt = "fdt-SEQ";
- loadables =
-#ifdef CONFIG_TARGET_IOT2050_A53_PG2
-
[PATCH 0/5] iot2050: 2023.10-rc1 fixes and cleanups
[resent, now that the list is working again]
Most of this comes late, but primarily as the required binman templating
changes were awaited, and the lots of regressions hit this board. I
really hope we can not only merge the minimally required patch 1 of this
series.
Jan
CC: Andrew Davis
CC: Manorit Chawdhry
CC: Simon Glass
Jan Kiszka (5):
boards: siemens: iot2050: Fix boot configuration
iot2050: Use binman in signing script
boards: siemens: iot2050: Unify PG1 and PG2/M.2 configurations again
doc: board: siemens: iot2050: Update build env vars
configs: iot2050: Enabled keyed autoboot
arch/arm/dts/k3-am65-iot2050-boot-image.dtsi | 155 +++---
board/siemens/iot2050/Kconfig | 30 +---
board/siemens/iot2050/MAINTAINERS | 3 +-
board/siemens/iot2050/board.c | 25 ++-
board/siemens/iot2050/iot2050.env | 2 +
...ot2050_pg1_defconfig => iot2050_defconfig} | 8 +-
configs/iot2050_pg2_defconfig | 150 -
doc/board/siemens/iot2050.rst | 31 ++--
include/configs/iot2050.h | 11 ++
tools/iot2050-sign-fw.sh | 11 +-
10 files changed, 158 insertions(+), 268 deletions(-)
rename configs/{iot2050_pg1_defconfig => iot2050_defconfig} (94%)
delete mode 100644 configs/iot2050_pg2_defconfig
--
2.35.3
[PATCH 4/5] doc: board: siemens: iot2050: Update build env vars
From: Jan Kiszka ATF is now called BL31, and OP-TEE since 3.21 suggests to use tee-raw.bin instead of (the still identical) tee-pager_v2.bin. Signed-off-by: Jan Kiszka --- doc/board/siemens/iot2050.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/board/siemens/iot2050.rst b/doc/board/siemens/iot2050.rst index 0df11a950a7..ee3c5c95846 100644 --- a/doc/board/siemens/iot2050.rst +++ b/doc/board/siemens/iot2050.rst @@ -66,8 +66,8 @@ U-Boot: .. code-block:: text - $ export ATF=/path/to/bl31.bin - $ export TEE=/path/to/tee-pager_v2.bin + $ export BL31=/path/to/bl31.bin + $ export TEE=/path/to/tee-raw.bin $ make iot2050_defconfig $ make -- 2.35.3
[PATCH 2/5] iot2050: Use binman in signing script
From: Jan Kiszka The underlying issue was fixed in the meantime. Also signing the U-Boot proper fit image now works. Just supporting custom cert templates remains a todo. Signed-off-by: Jan Kiszka --- CC: Simon Glass --- tools/iot2050-sign-fw.sh | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tools/iot2050-sign-fw.sh b/tools/iot2050-sign-fw.sh index 4d1d79498c2..3f953c09ed9 100755 --- a/tools/iot2050-sign-fw.sh +++ b/tools/iot2050-sign-fw.sh @@ -39,13 +39,8 @@ CERT_X509=$(mktemp .crt) openssl req -new -x509 -key $1 -nodes -outform DER -out $CERT_X509 -config $TEMP_X509 -sha512 cat $CERT_X509 tispl.bin > tispl.bin_signed -# currently broken in upstream -#source/tools/binman/binman replace -i flash.bin -f tispl.bin_signed blob@0x18 -dd if=tispl.bin_signed of=flash.bin bs=$((0x1000)) seek=$((0x18/0x1000)) conv=notrunc +source/tools/binman/binman replace -i flash.bin -f tispl.bin_signed fit@0x18 rm $TEMP_X509 $CERT_X509 -tools/mkimage -G $1 -r -o sha256,rsa4096 -F f...@0x38.fit -# currently broken in upstream -#source/tools/binman/binman replace -i flash.bin -f f...@0x38.fit fit@0x38 -dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) seek=$((0x38/0x1000)) conv=notrunc +source/tools/binman/binman sign -i flash.bin -k $1 -a sha256,rsa4096 fit@0x38 -- 2.35.3
[PATCH 1/5] boards: siemens: iot2050: Fix boot configuration
From: Jan Kiszka
The common env bits now come via ti_armv7_common.env, include it.
Futhermore restore the board-specific boot targets and their ordering
that is now enforced k3-wide differently. Finally, enable
CONFIG_LEGACY_IMAGE_FORMAT explicitly which got lost while turning
FIT_SIGNATURE on by default for k3 devices.
Fixes: 53873974 ("include: armv7: Enable distroboot across all configs")
Fixes: 4ae1a247 ("env: Make common bootcmd across all k3 devices")
Fixes: 86fab110 ("Kconfig: Enable FIT_SIGNATURE if ARM64")
Signed-off-by: Jan Kiszka
---
CC: Manorit Chawdhry
---
board/siemens/iot2050/iot2050.env | 2 ++
configs/iot2050_pg1_defconfig | 1 +
configs/iot2050_pg2_defconfig | 1 +
include/configs/iot2050.h | 11 +++
4 files changed, 15 insertions(+)
diff --git a/board/siemens/iot2050/iot2050.env
b/board/siemens/iot2050/iot2050.env
index 02958798b49..7fd836e6285 100644
--- a/board/siemens/iot2050/iot2050.env
+++ b/board/siemens/iot2050/iot2050.env
@@ -6,6 +6,8 @@
* Jan Kiszka
*/
+#include
+
usb_pgood_delay=900
watchdog_timeout_ms=CONFIG_WATCHDOG_TIMEOUT_MSECS
diff --git a/configs/iot2050_pg1_defconfig b/configs/iot2050_pg1_defconfig
index cc1b9673d79..391ab78d366 100644
--- a/configs/iot2050_pg1_defconfig
+++ b/configs/iot2050_pg1_defconfig
@@ -29,6 +29,7 @@ CONFIG_SPL_SPI=y
CONFIG_PCI=y
# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_OF_BOARD_SETUP=y
CONFIG_OF_SYSTEM_SETUP=y
CONFIG_DISTRO_DEFAULTS=y
diff --git a/configs/iot2050_pg2_defconfig b/configs/iot2050_pg2_defconfig
index c5741a4dae4..19c440732aa 100644
--- a/configs/iot2050_pg2_defconfig
+++ b/configs/iot2050_pg2_defconfig
@@ -29,6 +29,7 @@ CONFIG_SPL_SPI=y
CONFIG_PCI=y
# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_OF_BOARD_SETUP=y
CONFIG_OF_SYSTEM_SETUP=y
CONFIG_DISTRO_DEFAULTS=y
diff --git a/include/configs/iot2050.h b/include/configs/iot2050.h
index 2177e0dfe38..4968722d18f 100644
--- a/include/configs/iot2050.h
+++ b/include/configs/iot2050.h
@@ -15,6 +15,17 @@
#include
+/*
+ * This defines all MMC devices, even if the basic variant has no mmc1.
+ * The non-supported device will be removed from the boot targets during
+ * runtime, when that board was detected.
+ */
+#undef BOOT_TARGET_DEVICES
+#define BOOT_TARGET_DEVICES(func) \
+ func(MMC, mmc, 1) \
+ func(MMC, mmc, 0) \
+ BOOT_TARGET_USB(func)
+
#ifdef CONFIG_ENV_WRITEABLE_LIST
#define CFG_ENV_FLAGS_LIST_STATIC \
"board_uuid:sw,board_name:sw,board_serial:sw,board_a5e:sw," \
--
2.35.3
Re: [PATCH] xhci_register: Fix double free on failure
Thanks, my apologies.
On Wed, Jul 26, 2023 at 10:01 PM Marek Vasut wrote:
> On 7/24/23 21:45, Richard Habeeb wrote:
> > drivers/core/device.c will call `device_free()` after xhci_register
> > already frees the private device data. This can cause a crash later
> > during the boot process, observed on aarch64 RPi4b as a synchronous
> > exception. All callers of xhci_register use priv_auto, so this won't
> > lead to memory leaks.
> >
> > Signed-off-by: Richard Habeeb
> > ---
> >
> > drivers/usb/host/xhci.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index 9e33c5d855..5cacf0769e 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -1418,7 +1418,6 @@ int xhci_register(struct udevice *dev, struct
> > xhci_hccr *hccr,
> >
> >return 0;
> > err:
> > - free(ctrl);
> >debug("%s: failed, ret=%d\n", __func__, ret);
> >return ret;
> > }
>
> The patch is corrupted (tabs in original source replaced by spaces).
>
> Subject: tags should be 'usb: xhci:' .
>
> Please make sure to use git send-email and look at previous commits for
> subject tags next time .
>
> Both fixed and applied to usb/master , thanks.
>
[PATCH 5/5] bootstd: Init the size before reading extlinux file
The implementation in extlinux_pxe_getfile() does not pass a valid size to bootmeth_read_file(), so this can fail if the uninited value happens to be too small. Fix this. Signed-off-by: Simon Glass --- boot/bootmeth_pxe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/boot/bootmeth_pxe.c b/boot/bootmeth_pxe.c index ce986bd260d1..8d489a11aa40 100644 --- a/boot/bootmeth_pxe.c +++ b/boot/bootmeth_pxe.c @@ -31,6 +31,9 @@ static int extlinux_pxe_getfile(struct pxe_context *ctx, const char *file_path, int ret; addr = simple_strtoul(file_addr, NULL, 16); + + /* Allow up to 1GB */ + *sizep = 1 << 30; ret = bootmeth_read_file(info->dev, info->bflow, file_path, addr, sizep); if (ret) -- 2.41.0.487.g6d72f3e995-goog
[PATCH 4/5] bootstd: Init the size before reading the devicetree
The implementation in distro_efi_try_bootflow_files() does not pass a
valid size to bootmeth_common_read_file(), so this can fail if the
uninted value happens to be too small.
Fix this.
This was reported by someone but I cannot now find the email.
Signed-off-by: Simon Glass
---
boot/bootmeth_efi.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index bceec0d12ef3..1c9f2b1e2fe4 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#define EFI_DIRNAME"efi/boot/"
@@ -281,9 +282,12 @@ static int distro_efi_try_bootflow_files(struct udevice
*dev,
ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);
if (ret == -EALREADY)
bflow->flags = BOOTFLOWF_USE_PRIOR_FDT;
- if (!ret)
+ if (!ret) {
+ /* Limit FDT files to 4MB */
+ size = SZ_4M;
ret = bootmeth_common_read_file(dev, bflow, fname,
fdt_addr, &size);
+ }
}
if (*fname) {
--
2.41.0.487.g6d72f3e995-goog
[PATCH 2/5] bootstd: Use a function to detect network in EFI bootmeth
This checks for a network-based bootflow in two places, one of which is
less than ideal. Move the correct test into a function and use it in both
places.
Signed-off-by: Simon Glass
---
boot/bootmeth_efi.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index af31fbfc85db..e88e171ccc1b 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -94,6 +94,20 @@ static int get_efi_pxe_vci(char *str, int max_len)
return 0;
}
+/**
+ * bootmeth_uses_network() - check if the media device is Ethernet
+ *
+ * @bflow: Bootflow to check
+ * Returns: true if the media device is Ethernet, else false
+ */
+static bool bootmeth_uses_network(struct bootflow *bflow)
+{
+ const struct udevice *media = dev_get_parent(bflow->dev);
+
+ return IS_ENABLED(CONFIG_CMD_DHCP) &&
+ device_get_uclass_id(media) == UCLASS_ETH;
+}
+
static void set_efi_bootdev(struct blk_desc *desc, struct bootflow *bflow)
{
const struct udevice *media_dev;
@@ -354,11 +368,9 @@ static int distro_efi_read_bootflow_net(struct bootflow
*bflow)
static int distro_efi_read_bootflow(struct udevice *dev, struct bootflow
*bflow)
{
- const struct udevice *media = dev_get_parent(bflow->dev);
int ret;
- if (IS_ENABLED(CONFIG_CMD_DHCP) &&
- device_get_uclass_id(media) == UCLASS_ETH) {
+ if (bootmeth_uses_network(bflow)) {
/* we only support reading from one device, so ignore 'dev' */
ret = distro_efi_read_bootflow_net(bflow);
if (ret)
@@ -378,7 +390,7 @@ int distro_efi_boot(struct udevice *dev, struct bootflow
*bflow)
char cmd[50];
/* A non-zero buffer indicates the kernel is there */
- if (bflow->buf) {
+ if (!bootmeth_uses_network(bflow)) {
/* Set the EFI bootdev again, since reading an FDT loses it! */
if (bflow->blk) {
struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
--
2.41.0.487.g6d72f3e995-goog
[PATCH 3/5] bootstd: Avoid allocating memory for the EFI file
The current bootflow-iteration algorithm reads the bootflow file into
an allocated memory buffer so it can be examined. This works well in
most cases.
However, while the common case is that the first bootflow is immediately
booted, it is also possible just to scan for available bootflows, perhaps
selecting one to boot later.
Even with the common case, EFI bootflows can be quite large. It doesn't
make sense to read it into an allocated buffer when we have kernel_addr_t
providing a suitable address for it. Even if we do have plenty of malloc()
space available, it is a violation of U-Boot's lazy-init principle to
read the bootflow before it is needed.
So overall it seems better to make a change.
Adjust the logic to read just the size of the EFI file at first. Later,
when the bootflow is booted, read the rest of the file into the designated
kernel buffer.
Signed-off-by: Simon Glass
Reported-by: Da Xue
Reported-by: Vincent Stehlé
---
boot/bootmeth_efi.c | 50 ++---
1 file changed, 33 insertions(+), 17 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index e88e171ccc1b..bceec0d12ef3 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -143,13 +143,24 @@ static void set_efi_bootdev(struct blk_desc *desc, struct
bootflow *bflow)
efi_set_bootdev(dev_name, devnum_str, bflow->fname, bflow->buf, size);
}
-static int efiload_read_file(struct blk_desc *desc, struct bootflow *bflow)
+static int efiload_read_file(struct bootflow *bflow, ulong addr)
{
+ struct blk_desc *desc = NULL;
+ loff_t bytes_read;
int ret;
- ret = bootmeth_alloc_file(bflow, 0x200, 0x1);
+ if (bflow->blk)
+desc = dev_get_uclass_plat(bflow->blk);
+ ret = bootmeth_setup_fs(bflow, desc);
+ if (ret)
+ return log_msg_ret("set", ret);
+
+ ret = fs_read(bflow->fname, addr, 0, bflow->size, &bytes_read);
if (ret)
return log_msg_ret("read", ret);
+ bflow->buf = map_sysmem(addr, bflow->size);
+
+ set_efi_bootdev(desc, bflow);
return 0;
}
@@ -223,7 +234,18 @@ static int distro_efi_get_fdt_name(char *fname, int size,
int seq)
return 0;
}
-static int distro_efi_read_bootflow_file(struct udevice *dev,
+/*
+ * distro_efi_try_bootflow_files() - Check that files are present
+ *
+ * This reads any FDT file and checks whether the bootflow file is present, for
+ * later reading. We avoid reading the bootflow now, since it is likely large,
+ * it may take a long time and we want to avoid needing to allocate memory for
+ * it
+ *
+ * @dev: bootmeth device to use
+ * @bflow: bootflow to update
+ */
+static int distro_efi_try_bootflow_files(struct udevice *dev,
struct bootflow *bflow)
{
struct blk_desc *desc = NULL;
@@ -247,9 +269,8 @@ static int distro_efi_read_bootflow_file(struct udevice
*dev,
if (ret)
return log_msg_ret("try", ret);
- ret = efiload_read_file(desc, bflow);
- if (ret)
- return log_msg_ret("read", ret);
+ /* Since we can access the file, let's call it ready */
+ bflow->state = BOOTFLOWST_READY;
fdt_addr = env_get_hex("fdt_addr_r", 0);
@@ -376,7 +397,7 @@ static int distro_efi_read_bootflow(struct udevice *dev,
struct bootflow *bflow)
if (ret)
return log_msg_ret("net", ret);
} else {
- ret = distro_efi_read_bootflow_file(dev, bflow);
+ ret = distro_efi_try_bootflow_files(dev, bflow);
if (ret)
return log_msg_ret("blk", ret);
}
@@ -388,17 +409,13 @@ int distro_efi_boot(struct udevice *dev, struct bootflow
*bflow)
{
ulong kernel, fdt;
char cmd[50];
+ int ret;
- /* A non-zero buffer indicates the kernel is there */
+ kernel = env_get_hex("kernel_addr_r", 0);
if (!bootmeth_uses_network(bflow)) {
- /* Set the EFI bootdev again, since reading an FDT loses it! */
- if (bflow->blk) {
- struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
-
- set_efi_bootdev(desc, bflow);
- }
-
- kernel = (ulong)map_to_sysmem(bflow->buf);
+ ret = efiload_read_file(bflow, kernel);
+ if (ret)
+ return log_msg_ret("read", ret);
/*
* use the provided device tree if available, else fall back to
@@ -417,7 +434,6 @@ int distro_efi_boot(struct udevice *dev, struct bootflow
*bflow)
* But this is the same behaviour for distro boot, so it can be
* fixed here.
*/
- kernel = env_get_hex("kernel_addr_r", 0);
fdt = env_get_hex("fdt_addr_r", 0);
}
--
2.41.0.487.g6d72f3e995-goog
[PATCH 1/5] bootflow: Export setup_fs()
This function is used in some bootmeth implementations. Export it.
Signed-off-by: Simon Glass
---
boot/bootmeth-uclass.c | 23 ++-
include/bootmeth.h | 13 +
2 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/boot/bootmeth-uclass.c b/boot/bootmeth-uclass.c
index eeded08dd428..175eb1de5e1e 100644
--- a/boot/bootmeth-uclass.c
+++ b/boot/bootmeth-uclass.c
@@ -240,18 +240,7 @@ int bootmeth_set_order(const char *order_str)
return 0;
}
-/**
- * setup_fs() - Set up read to read a file
- *
- * We must redo the setup before each filesystem operation. This function
- * handles that, including setting the filesystem type if a block device is not
- * being used
- *
- * @bflow: Information about file to try
- * @desc: Block descriptor to read from (NULL if not a block device)
- * Return: 0 if OK, -ve on error
- */
-static int setup_fs(struct bootflow *bflow, struct blk_desc *desc)
+int bootmeth_setup_fs(struct bootflow *bflow, struct blk_desc *desc)
{
int ret;
@@ -288,7 +277,7 @@ int bootmeth_try_file(struct bootflow *bflow, struct
blk_desc *desc,
log_debug(" %s - err=%d\n", path, ret);
/* Sadly FS closes the file after fs_size() so we must redo this */
- ret2 = setup_fs(bflow, desc);
+ ret2 = bootmeth_setup_fs(bflow, desc);
if (ret2)
return log_msg_ret("fs", ret2);
@@ -337,14 +326,14 @@ int bootmeth_alloc_other(struct bootflow *bflow, const
char *fname,
if (bflow->blk)
desc = dev_get_uclass_plat(bflow->blk);
- ret = setup_fs(bflow, desc);
+ ret = bootmeth_setup_fs(bflow, desc);
if (ret)
return log_msg_ret("fs", ret);
ret = fs_size(path, &size);
log_debug(" %s - err=%d\n", path, ret);
- ret = setup_fs(bflow, desc);
+ ret = bootmeth_setup_fs(bflow, desc);
if (ret)
return log_msg_ret("fs", ret);
@@ -369,7 +358,7 @@ int bootmeth_common_read_file(struct udevice *dev, struct
bootflow *bflow,
if (bflow->blk)
desc = dev_get_uclass_plat(bflow->blk);
- ret = setup_fs(bflow, desc);
+ ret = bootmeth_setup_fs(bflow, desc);
if (ret)
return log_msg_ret("fs", ret);
@@ -379,7 +368,7 @@ int bootmeth_common_read_file(struct udevice *dev, struct
bootflow *bflow,
if (size > *sizep)
return log_msg_ret("spc", -ENOSPC);
- ret = setup_fs(bflow, desc);
+ ret = bootmeth_setup_fs(bflow, desc);
if (ret)
return log_msg_ret("fs", ret);
diff --git a/include/bootmeth.h b/include/bootmeth.h
index c3df9702e871..7cb7da33deaa 100644
--- a/include/bootmeth.h
+++ b/include/bootmeth.h
@@ -262,6 +262,19 @@ int bootmeth_setup_iter_order(struct bootflow_iter *iter,
bool include_global);
*/
int bootmeth_set_order(const char *order_str);
+/**
+ * bootmeth_setup_fs() - Set up read to read a file
+ *
+ * We must redo the setup before each filesystem operation. This function
+ * handles that, including setting the filesystem type if a block device is not
+ * being used
+ *
+ * @bflow: Information about file to try
+ * @desc: Block descriptor to read from (NULL if not a block device)
+ * Return: 0 if OK, -ve on error
+ */
+int bootmeth_setup_fs(struct bootflow *bflow, struct blk_desc *desc);
+
/**
* bootmeth_try_file() - See we can access a given file
*
--
2.41.0.487.g6d72f3e995-goog
[PATCH 0/5] bootstd: Fix some reported problems
This little series fixes a few problems reported over the past few months. The main changes is to the EFI bootmeth, which now delays reading the .efi file until it is actually needed. Simon Glass (5): bootflow: Export setup_fs() bootstd: Use a function to detect network in EFI bootmeth bootstd: Avoid allocating memory for the EFI file bootstd: Init the size before reading the devicetree bootstd: Init the size before reading extlinux file boot/bootmeth-uclass.c | 23 - boot/bootmeth_efi.c| 76 ++ boot/bootmeth_pxe.c| 3 ++ include/bootmeth.h | 13 4 files changed, 76 insertions(+), 39 deletions(-) -- 2.41.0.487.g6d72f3e995-goog
Re: [PATCH v2 05/12] firmware: scmi: install base protocol to SCMI agent
Hi,
On Wed, 26 Jul 2023 at 19:07, AKASHI Takahiro
wrote:
>
> Hi Simon,
>
> Thank you for your extensive review.
Cheers.
>
> On Wed, Jul 26, 2023 at 06:50:23PM -0600, Simon Glass wrote:
> > Hi,
> >
> > On Wed, 26 Jul 2023 at 02:39, AKASHI Takahiro
> > wrote:
> > >
> > > SCMI base protocol is mandatory, and once SCMI node is found in a device
> > > tree, the protocol handle (udevice) is unconditionally installed to
> > > the agent. Then basic information will be retrieved from SCMI server via
> > > the protocol and saved into the agent instance's local storage.
> > >
> > > Signed-off-by: AKASHI Takahiro
> > > ---
> > > v2
> > > * use helper functions, removing direct uses of ops
> > > ---
> > > drivers/firmware/scmi/scmi_agent-uclass.c | 116 ++
> > > include/scmi_agent-uclass.h | 70 -
> > > 2 files changed, 184 insertions(+), 2 deletions(-)
> > >
> >
> > Reviewed-by: Simon Glass
> >
[..]
> > > +static inline u32 scmi_agent_id(struct udevice *dev)
> > > +{
> > > + return ((struct scmi_agent_priv
> > > *)dev_get_uclass_plat(dev))->agent_id;
> > > +}
> >
> > Why these helper functions? It seems better to avoid inlining access
> > to dev_get_uclass_plat(). Are you worried about exposing the priv
> > struct to clients?
>
> Well, simply wanted to avoid repeating
> "((struct scmi_agent_priv *)dev_get_uclass_plat(dev))->)" in the code.
That's fine, so long as it would not be better for the caller to
obtain the priv pointer and just access the fields in the struct,
Regards,
Simon
Re: [PATCH] xhci_register: Fix double free on failure
On 7/24/23 21:45, Richard Habeeb wrote:
drivers/core/device.c will call `device_free()` after xhci_register
already frees the private device data. This can cause a crash later
during the boot process, observed on aarch64 RPi4b as a synchronous
exception. All callers of xhci_register use priv_auto, so this won't
lead to memory leaks.
Signed-off-by: Richard Habeeb
---
drivers/usb/host/xhci.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 9e33c5d855..5cacf0769e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1418,7 +1418,6 @@ int xhci_register(struct udevice *dev, struct
xhci_hccr *hccr,
return 0;
err:
- free(ctrl);
debug("%s: failed, ret=%d\n", __func__, ret);
return ret;
}
The patch is corrupted (tabs in original source replaced by spaces).
Subject: tags should be 'usb: xhci:' .
Please make sure to use git send-email and look at previous commits for
subject tags next time .
Both fixed and applied to usb/master , thanks.
Re: [PATCH] riscv: Support riscv64 image type
Hi Rick, On Thu, Jul 27, 2023 at 09:27:31AM +0800, Rick Chen wrote: > > Hi Rick, > > > > On Wed, 19 Apr 2023 at 00:56, Rick Chen wrote: > > > > > > Hi Simon, > > > > > > > Hi Rick, > > > > > > > > On Mon, 10 Apr 2023 at 01:26, Rick Chen wrote: > > > > > > > > > > Allow U-Boot to load 32 or 64 bits RISC-V Kernel Image > > > > > distinguishly. It helps to avoid someone maybe make a mistake > > > > > to run 32-bit U-Boot to load 64-bit kernel. > > > > > > > > > > Signed-off-by: Rick Chen > > > > > > > > > > --- > > > > > The patchset is based on Simon's patch: > > > > > riscv: Add a 64-bit image type > > > > > --- > > > > > --- > > > > > arch/riscv/include/asm/u-boot.h | 4 > > > > > cmd/booti.c | 2 +- > > > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > > > Reviewed-by: Simon Glass > > > > > > > > I don't know much about RISC-V, but I assume U-Boot is able to do this > > > > successfully? Does it not need to switch modes first? > > > > > > No, it is not need to switch modes as far as I know. > > > Here only provide a check mechanism just like arm to see if loader and > > > OS are match > > > > > > But This patch is for bootm flow. > > > Maybe I still need to check if it is necessary to prepare a patch for > > > binman flow ? > > > /arch/riscv/dts/binman.dtsi > > > arch = "riscv"; > > > > > > maybe provide another binman64.dtsi for arch="riscv64" > > > > Yes I think that is needed too. Are you going to update this patch, or > > send a second one? > > Hi Simon, > > OK. > > HI Leo, > > I am a little busy on other things.Can you help to update this patch ? > No problem! I will send a v2 patch ASAP. Best regards, Leo > Thanks. > Rick > > > > > Regards, > > Simon
Re: [PATCH] riscv: Support riscv64 image type
> Hi Rick, > > On Wed, 19 Apr 2023 at 00:56, Rick Chen wrote: > > > > Hi Simon, > > > > > Hi Rick, > > > > > > On Mon, 10 Apr 2023 at 01:26, Rick Chen wrote: > > > > > > > > Allow U-Boot to load 32 or 64 bits RISC-V Kernel Image > > > > distinguishly. It helps to avoid someone maybe make a mistake > > > > to run 32-bit U-Boot to load 64-bit kernel. > > > > > > > > Signed-off-by: Rick Chen > > > > > > > > --- > > > > The patchset is based on Simon's patch: > > > > riscv: Add a 64-bit image type > > > > --- > > > > --- > > > > arch/riscv/include/asm/u-boot.h | 4 > > > > cmd/booti.c | 2 +- > > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > > > Reviewed-by: Simon Glass > > > > > > I don't know much about RISC-V, but I assume U-Boot is able to do this > > > successfully? Does it not need to switch modes first? > > > > No, it is not need to switch modes as far as I know. > > Here only provide a check mechanism just like arm to see if loader and > > OS are match > > > > But This patch is for bootm flow. > > Maybe I still need to check if it is necessary to prepare a patch for > > binman flow ? > > /arch/riscv/dts/binman.dtsi > > arch = "riscv"; > > > > maybe provide another binman64.dtsi for arch="riscv64" > > Yes I think that is needed too. Are you going to update this patch, or > send a second one? Hi Simon, OK. HI Leo, I am a little busy on other things.Can you help to update this patch ? Thanks. Rick > > Regards, > Simon
Re: [PATCH v2 05/12] firmware: scmi: install base protocol to SCMI agent
Hi Simon,
Thank you for your extensive review.
On Wed, Jul 26, 2023 at 06:50:23PM -0600, Simon Glass wrote:
> Hi,
>
> On Wed, 26 Jul 2023 at 02:39, AKASHI Takahiro
> wrote:
> >
> > SCMI base protocol is mandatory, and once SCMI node is found in a device
> > tree, the protocol handle (udevice) is unconditionally installed to
> > the agent. Then basic information will be retrieved from SCMI server via
> > the protocol and saved into the agent instance's local storage.
> >
> > Signed-off-by: AKASHI Takahiro
> > ---
> > v2
> > * use helper functions, removing direct uses of ops
> > ---
> > drivers/firmware/scmi/scmi_agent-uclass.c | 116 ++
> > include/scmi_agent-uclass.h | 70 -
> > 2 files changed, 184 insertions(+), 2 deletions(-)
> >
>
> Reviewed-by: Simon Glass
>
> > diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c
> > b/drivers/firmware/scmi/scmi_agent-uclass.c
> > index 2244fcf487e8..279c2c218913 100644
> > --- a/drivers/firmware/scmi/scmi_agent-uclass.c
> > +++ b/drivers/firmware/scmi/scmi_agent-uclass.c
> > @@ -57,6 +57,9 @@ struct udevice *scmi_get_protocol(struct udevice *dev,
> > }
> >
> > switch (id) {
> > + case SCMI_PROTOCOL_ID_BASE:
> > + proto = priv->base_dev;
> > + break;
> > case SCMI_PROTOCOL_ID_CLOCK:
> > proto = priv->clock_dev;
> > break;
> > @@ -101,6 +104,9 @@ static int scmi_add_protocol(struct udevice *dev,
> > }
> >
> > switch (proto_id) {
> > + case SCMI_PROTOCOL_ID_BASE:
> > + priv->base_dev = proto;
> > + break;
> > case SCMI_PROTOCOL_ID_CLOCK:
> > priv->clock_dev = proto;
> > break;
> > @@ -229,6 +235,83 @@ int devm_scmi_process_msg(struct udevice *dev, struct
> > scmi_msg *msg)
> > return scmi_process_msg(parent, priv->channel, msg);
> > }
> >
> > +/**
> > + * scmi_fill_base_info - get base information about SCMI server
> > + * @agent: SCMI agent device
> > + * @dev: SCMI protocol device
> > + *
> > + * By using Base protocol commands, collect the base information
> > + * about SCMI server.
> > + *
> > + * Return: 0 on success, error code otherwise
> > + */
> > +static int scmi_fill_base_info(struct udevice *agent, struct udevice *dev)
> > +{
> > + struct scmi_agent_priv *priv = dev_get_uclass_plat(agent);
> > + int ret;
> > +
> > + ret = scmi_base_protocol_version(dev, &priv->version);
> > + if (ret) {
> > + dev_err(dev, "protocol_version() failed (%d)\n", ret);
> > + return ret;
> > + }
> > + /* check for required version */
> > + if (priv->version < SCMI_BASE_PROTOCOL_VERSION) {
> > + dev_err(dev, "base protocol version (%d) lower than
> > expected\n",
> > + priv->version);
> > + return -EPROTO;
> > + }
> > +
> > + ret = scmi_base_protocol_attrs(dev, &priv->num_agents,
> > + &priv->num_protocols);
> > + if (ret) {
> > + dev_err(dev, "protocol_attrs() failed (%d)\n", ret);
> > + return ret;
> > + }
> > + ret = scmi_base_discover_vendor(dev, priv->vendor);
> > + if (ret) {
> > + dev_err(dev, "base_discover_vendor() failed (%d)\n", ret);
> > + return ret;
> > + }
> > + ret = scmi_base_discover_sub_vendor(dev, priv->sub_vendor);
> > + if (ret) {
> > + if (ret != -EOPNOTSUPP) {
> > + dev_err(dev, "base_discover_sub_vendor() failed
> > (%d)\n",
> > + ret);
> > + return ret;
> > + }
> > + strcpy(priv->sub_vendor, "NA");
> > + }
> > + ret = scmi_base_discover_impl_version(dev, &priv->impl_version);
> > + if (ret) {
> > + dev_err(dev, "base_discover_impl_version() failed (%d)\n",
> > + ret);
> > + return ret;
> > + }
> > +
> > + priv->agent_id = 0x; /* to avoid false claim */
> > + ret = scmi_base_discover_agent(dev, 0x,
> > + &priv->agent_id, priv->agent_name);
> > + if (ret) {
> > + if (ret != -EOPNOTSUPP) {
> > + dev_err(dev,
> > + "base_discover_agent() failed for myself
> > (%d)\n",
> > + ret);
> > + return ret;
> > + }
> > + priv->agent_name[0] = '\0';
> > + }
> > +
> > + ret = scmi_base_discover_list_protocols(dev, &priv->protocols);
> > + if (ret != priv->num_protocols) {
> > + dev_err(dev, "base_discover_list_protocols() failed (%d)\n",
> > + ret);
> > + return ret;
> > + }
> > +
Re: [PATCH 1/4] rockchip: rk3308: fix board_debug_uart_init
On Thu, Jul 27, 2023 at 09:00:52AM +0800, Kever Yang wrote:
> Hi Tom,
>
> I have reply the review tag to the list last week, but this mail does
> not appear at the patchwork system[1],
>
> did you met this kind of issue and do you know how to fix it?
There've been a few hiccups with the mailing list of late, hopefully
your message will get re-tried and show up in patchwork. Otherwise you
may have to manually add tags back when putting together a pull request,
sorry about that.
>
>
> Thanks,
>
> - Kever
>
> [1]
> https://patchwork.ozlabs.org/project/uboot/patch/gv1pr08mb801036b040f257c97c652296e5...@gv1pr08mb8010.eurprd08.prod.outlook.com/
>
> On 2023/7/21 17:05, Kever Yang wrote:
> >
> > On 2023/7/15 18:19, Pegorer Massimo wrote:
> > > Definition of function board_debug_uart_init() must be under
> > > CONFIG_DEBUG_UART_BOARD_INIT and not under CONFIG_DEBUG_UART,
> > > as it was: see debug_uart.h. In this way the debug uart can
> > > be used but its board-specific initialization skipped by
> > > configuration, if useless.
> > >
> > > Signed-off-by: Massimo Pegorer
> > Reviewed-by: Kever Yang
> >
> > Thanks,
> > - Kever
> > > ---
> > > arch/arm/mach-rockchip/rk3308/rk3308.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/mach-rockchip/rk3308/rk3308.c
> > > b/arch/arm/mach-rockchip/rk3308/rk3308.c
> > > index dd9109b7c3..5763604dc3 100644
> > > --- a/arch/arm/mach-rockchip/rk3308/rk3308.c
> > > +++ b/arch/arm/mach-rockchip/rk3308/rk3308.c
> > > @@ -174,7 +174,7 @@ int rk_board_init(void)
> > > return 0;
> > > }
> > > -#if defined(CONFIG_DEBUG_UART)
> > > +#ifdef CONFIG_DEBUG_UART_BOARD_INIT
> > > __weak void board_debug_uart_init(void)
> > > {
> > > static struct rk3308_grf * const grf = (void *)GRF_BASE;
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] fs/erofs: Remove an unnecessary assertion
Thanks greatly for the speedy turnaround on this patch!
On 7/25/23 22:56, Yifan Zhao wrote:
Fixes: 3a21e92fc255 ("fs/erofs: Introduce new features including ztailpacking,
fragments and dedupe")
Signed-off-by: Yifan Zhao
Tested-by: Sam Edwards
Re: [PATCH 1/4] rockchip: rk3308: fix board_debug_uart_init
Hi Tom,
I have reply the review tag to the list last week, but this mail
does not appear at the patchwork system[1],
did you met this kind of issue and do you know how to fix it?
Thanks,
- Kever
[1]
https://patchwork.ozlabs.org/project/uboot/patch/gv1pr08mb801036b040f257c97c652296e5...@gv1pr08mb8010.eurprd08.prod.outlook.com/
On 2023/7/21 17:05, Kever Yang wrote:
On 2023/7/15 18:19, Pegorer Massimo wrote:
Definition of function board_debug_uart_init() must be under
CONFIG_DEBUG_UART_BOARD_INIT and not under CONFIG_DEBUG_UART,
as it was: see debug_uart.h. In this way the debug uart can
be used but its board-specific initialization skipped by
configuration, if useless.
Signed-off-by: Massimo Pegorer
Reviewed-by: Kever Yang
Thanks,
- Kever
---
arch/arm/mach-rockchip/rk3308/rk3308.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/rk3308/rk3308.c
b/arch/arm/mach-rockchip/rk3308/rk3308.c
index dd9109b7c3..5763604dc3 100644
--- a/arch/arm/mach-rockchip/rk3308/rk3308.c
+++ b/arch/arm/mach-rockchip/rk3308/rk3308.c
@@ -174,7 +174,7 @@ int rk_board_init(void)
return 0;
}
-#if defined(CONFIG_DEBUG_UART)
+#ifdef CONFIG_DEBUG_UART_BOARD_INIT
__weak void board_debug_uart_init(void)
{
static struct rk3308_grf * const grf = (void *)GRF_BASE;
Re: [PATCH v2 02/12] firmware: scmi: implement SCMI base protocol
On Wed, 26 Jul 2023 at 02:38, AKASHI Takahiro wrote: > > SCMI base protocol is mandatory according to the SCMI specification. > > With this patch, SCMI base protocol can be accessed via SCMI transport > layers. All the commands, except SCMI_BASE_NOTIFY_ERRORS, are supported. > This is because U-Boot doesn't support interrupts and the current transport > layers are not able to handle asynchronous messages properly. > > Signed-off-by: AKASHI Takahiro > --- > v2 > * add helper functions, removing direct uses of ops > * add function descriptions for each of functions in ops > --- > drivers/firmware/scmi/Makefile | 1 + > drivers/firmware/scmi/base.c | 637 + > include/dm/uclass-id.h | 1 + > include/scmi_protocols.h | 345 ++ > 4 files changed, 984 insertions(+) > create mode 100644 drivers/firmware/scmi/base.c Reviewed-by: Simon Glass
Re: [PATCH v2 01/12] scmi: refactor the code to hide a channel from devices
On Wed, 26 Jul 2023 at 02:38, AKASHI Takahiro
wrote:
>
> The commit 85dc58289238 ("firmware: scmi: prepare uclass to pass channel
> reference") added an explicit parameter, channel, but it seems to make
> the code complex.
>
> Hiding this parameter will allow for adding a generic (protocol-agnostic)
> helper function, i.e. for PROTOCOL_VERSION, in a later patch.
>
> Signed-off-by: AKASHI Takahiro
> ---
> v2
> * new patch
> ---
> drivers/clk/clk_scmi.c| 27 ++---
> drivers/firmware/scmi/scmi_agent-uclass.c | 74 ++-
> drivers/power/regulator/scmi_regulator.c | 27 +++--
> drivers/reset/reset-scmi.c| 19 +-
> include/scmi_agent.h | 15 +++--
> 5 files changed, 86 insertions(+), 76 deletions(-)
>
Reviewed-by: Simon Glass
Re: [RFC PATCH 01/10] remoteproc: move resource table definition
On Tue, 25 Jul 2023 at 08:08, Tanmay Shah wrote: > > Resource table is defined in multiple files. Instead move > definition to remoteproc header file and include header file > in multiple c files where resource table definition is needed. > > Signed-off-by: Tanmay Shah > --- > drivers/remoteproc/rproc-elf-loader.c | 33 --- > drivers/remoteproc/rproc-uclass.c | 7 -- > include/remoteproc.h | 33 +++ > 3 files changed, 33 insertions(+), 40 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 1/5] x86: fsp: Use mtrr_set_next_var() for graphics memory
Hi Bin, On Tue, 25 Jul 2023 at 07:43, Bin Meng wrote: > > Hi Simon, > > On Mon, Jul 24, 2023 at 6:14 AM Simon Glass wrote: > > > > Hi Bin, > > > > On Sun, 23 Jul 2023 at 09:50, Bin Meng wrote: > > > > > > Hi Simon, > > > > > > On Sun, Jul 23, 2023 at 11:43 AM Simon Glass wrote: > > > > > > > > Hi Bin, > > > > > > > > On Fri, 21 Jul 2023 at 10:12, Bin Meng wrote: > > > > > > > > > > At present this uses mtrr_add_request() & mtrr_commit() combination > > > > > to program the MTRR for graphics memory. This usage has two major > > > > > issues as below: > > > > > > > > > > - mtrr_commit() will re-initialize all MTRR registers from index 0, > > > > > using the settings previously added by mtrr_add_request() and saved > > > > > in gd->arch.mtrr_req[], which won't cause any issue but is > > > > > unnecessary > > > > > - The way such combination works is based on the assumption that > > > > > U-Boot > > > > > has full control with MTRR programming (e.g.: U-Boot without any > > > > > blob > > > > > that does all low-level initialization on its own, or using FSP2 > > > > > which > > > > > does not touch MTRR), but this is not the case with FSP. FSP > > > > > programs > > > > > some MTRRs during its execution but U-Boot does not have the > > > > > settings > > > > > saved in gd->arch.mtrr_req[] and when doing mtrr_commit() it will > > > > > corrupt what was already programmed previously. > > > > > > > > > > Correct this to use mtrr_set_next_var() instead. > > > > > > > > > > Signed-off-by: Bin Meng > > > > > --- > > > > > > > > > > arch/x86/lib/fsp/fsp_graphics.c | 3 +-- > > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > Thanks for looking into this. The series works fine on link. I suspect > > > > it will be find on samus too, but I cannot test right now. Sadly > > > > minnowmax is also dead right now but I hope to fix it soon. I don't > > > > expect any problems there. > > > > > > > > However, for coral, this first patch breaks the mtrrs. With master we > > > > get: > > > > > > > > => mtrr > > > > CPU 0: > > > > Reg Valid Write-type Base ||Mask ||Size || > > > > 0 Y Back fef0 0078 > > > > 0008 > > > > 1 Y Back fef8 007c > > > > 0004 > > > > 2 Y Back 007f8000 > > > > 8000 > > > > 3 Y Combine b000 007ff000 > > > > 1000 > > > > 4 Y Back 0001 007f8000 > > > > 8000 > > > > 5 N Uncacheable > > > > 0080 > > > > 6 N Uncacheable > > > > 0080 > > > > 7 N Uncacheable > > > > 0080 > > > > 8 N Uncacheable > > > > 0080 > > > > 9 N Uncacheable > > > > 0080 > > > > > > > > with this patch on coral we get: > > > > > > > > => mtrr > > > > CPU 0: > > > > Reg Valid Write-type Base ||Mask ||Size || > > > > 0 Y Back fef0 0078 > > > > 0008 > > > > 1 Y Back fef8 007c > > > > 0004 > > > > 2 Y Combine b000 007ff000 > > > > 1000 > > > > 3 N Uncacheable > > > > 0080 > > > > > > > > At present coral expects to handle the MTRRs itself, and it seems that > > > > perhaps the APL FSPv2 does not? Do we need a new Kconfig for dealing > > > > with FSPv2 perhaps? > > > > > > I am a little bit confused. The comment in arch/x86/lib/fsp/fsp_dram.c:: > > > dram_init_banksize() says: > > > > > > /* > > > * For FSP1, the system memory and reserved memory used by FSP are > > > * already programmed in the MTRR by FSP. Also it is observed that > > > * FSP on Intel Queensbay platform reports the TSEG memory range > > > * that has the same RES_MEM_RESERVED resource type whose address > > > * is programmed by FSP to be near the top of 4 GiB space, which > > > is > > > * not what we want for DRAM. > > > * > > > * However it seems FSP2's behavior is different. We need to add > > > the > > > * DRAM range in MTRR otherwise the boot process goes very slowly, > > > * which was observed on Chromebook Coral with FSP2. > > > */ > > > > > > So on Coral with FSP2, U-Boot programs the MTTR by itself. > > > > > > In this dram_init_banksize(), it calls mtrr_add_request() 3 times, 2 > > > of which should be what you were seeing as 2 and 4 below: > > > > > > > 2 Y Back 007f8000 > > > > 8000 > > > > 3 Y Combine
Re: Strange construct in binman description
Hi Tim,
On Mon, 24 Jul 2023 at 08:53, Simon Glass wrote:
>
> Hi,
>
> On Sun, 23 Jul 2023 at 03:00, Marcel Ziswiler
> wrote:
> >
> > Hi Simon
> >
> > On Jul 23, 2023 05:48, Simon Glass wrote:
> >
> > Hi Marcel,
> >
> > I just noticed this in an imx8 description:
> >
> > binman_configuration: @config-SEQ {
> >
> >
> > I remember having stumbled over this before but I do not remember any exact
> > details, sorry.
> >
> > Since this is a generator node, binman blindly generates a phandle for
> > each not it generates. This means that if there is more than one
> > config node, then they will have duplicate phandles.
> >
> > I have sent a series to correct this, but it fails the build on:
> > imx8mm_venice imx8mn_venice imx8mp_venice
> >
> >
> > Ah, now I get it. You mixed up venice (Gate works) with verdin (Toradex).
> >
> > I am a bit unsure about what this is supposed to do. Could you please
> > take a look?
> >
> >
> > I'm more than happy to do that but you are probably much better off
> > enquiring Tim Harvey from Gateworks about this. I put him on CC.
>
> Tim, any thoughts on this please?
Can you please weigh in on this one?
Regards,
Simon
Re: [PATCH] remoteproc: uclass: Clean up a return
On Wed, 26 Jul 2023 at 10:50, Dan Carpenter wrote: > > We know that "pa" is non-NULL so it's nicer to just return zero instead > of return !pa. This has no effect on runtime behavior. > > Signed-off-by: Dan Carpenter > --- > drivers/remoteproc/rproc-uclass.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH 2/5] usb: dwc2: Use regulator_set_enable_if_allowed
On Wed, 19 Jul 2023 at 15:21, Jonas Karlman wrote:
>
> With the commit 4fcba5d556b4 ("regulator: implement basic reference
> counter") the return value of regulator_set_enable may be EALREADY or
> EBUSY for fixed/gpio regulators.
>
> Change to use the more relaxed regulator_set_enable_if_allowed to
> continue if regulator already was enabled or disabled.
>
> Signed-off-by: Jonas Karlman
> ---
> drivers/usb/host/dwc2.c | 14 ++
> 1 file changed, 6 insertions(+), 8 deletions(-)
Reviewed-by: Simon Glass
Tested-by: Simon Glass # rockpro64-rk3399
Re: [PATCH] video: Add parentheses around VNBYTES() macro
On Wed, 26 Jul 2023 at 00:54, Dan Carpenter wrote: > > The VNBYTES() macro needs to have parentheses to prevent some (harmless) > macro expansion bugs. The VNBYTES() macro is used like this: > > VID_TO_PIXEL(x) * VNBYTES(vid_priv->bpix) > > The * operation is done before the / operation. It still ends up with > the same results, but it's not ideal. > > Signed-off-by: Dan Carpenter > --- > include/video.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I'd argue that doing * before / is better, since it might be more accurate. But I don't know if it actually matters. Reviewed-by: Simon Glass
Re: [PATCH 5/9] board_f: Fix corruption of relocaddr
Hi Devarsh,
On Wed, 26 Jul 2023 at 05:09, Devarsh Thakkar wrote:
>
> Hi Simon,
>
> On 26/07/23 02:58, Simon Glass wrote:
> > Hi Devarsh,
> >
> > On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar wrote:
> >>
> >> Hi Simon,
> >>
> >> On 24/07/23 20:22, Simon Glass wrote:
> >>> When the video framebuffer comes from the bloblist, we should not change
> >>> relocaddr to this address, since it interfers with the normal memory
> >>> allocation.
> >>>
> >>> This fixes a boot loop in qemu-x86_64
> >>>
> >>> Signed-off-by: Simon Glass
> >>> Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to
> >>> u-boot")
> >>> ---
> >>>
> >>> common/board_f.c | 1 -
> >>> 1 file changed, 1 deletion(-)
> >>>
> >>> diff --git a/common/board_f.c b/common/board_f.c
> >>> index 7d2c380e91e2..5c8646b22283 100644
> >>> --- a/common/board_f.c
> >>> +++ b/common/board_f.c
> >>> @@ -419,7 +419,6 @@ static int reserve_video(void)
> >>> if (!ho)
> >>> return log_msg_ret("blf", -ENOENT);
> >>> video_reserve_from_bloblist(ho);
> >>> - gd->relocaddr = ho->fb;
> >>
> >> I think this change was done as relocaddr pointer was required to be
> >> updated
> >> to move after frame-buffer reserved area to ensure that any further memory
> >> reservations done using gd->relocaddr (for e.g. in
> >> reserve_trace/uboot/malloc)
> >> don't overlap with frame-buffer reserved area passed from blob, so I think
> >> removing this line may cause further memory reservations to overlap with
> >> reserved framebuffer.
> >>
> >> Could you please confirm?
> >
> > SPL and U-Boot have different memory layouts. The current code is
> > interrupting U-Boot's careful building up of chunks of memory used for
> > different purposes.
> >
>
> But it is possible they could be using similar memory layout for some
> components like framebuffer.
> For e.g. in our case we are using same video_reserve func in A53 SPL too
> and which allocates framebuffer from gd->relocaddr as seen here:
>
> https://source.denx.de/u-boot/u-boot/-/blob/v2023.10-rc1/common/board_f.c?ref_type=tags#L427
Even if it is similar, the point is that U-Boot proper needs to do its
own allocation stuff.
Of course, if SPL sets up the video, it will provide the framebuffer
address, but only that. The other addresses need to be done using the
normal mechanism.
>
> > The video memory has already been allocated by SPL, so we don't need
> > to insert a new one here, as your code demonstrates.
> >
>
> Agreed.
>
> > But also we have no way of knowing if it is legal to relocate U-Boot
> > (and various other things) just below the frame buffer chosen by SPL.
> >
>
> Yes, so i suppose your case is that framebuffer address which is being passed
> by SPL is totally disjoint and too far away from gd->relocaddr, for e.g. it is
> at the start (or bottom of DDR) and doesn't interfere with gd->relocaddr in
> any manner since relocaddr points to ramtop (i.e. near to end address of DDR).
>
> In that case I agree it doesn't make sense to move relocaddr to ho->fb.
>
> But for the scenario where gd->relocaddr and ho->fb are nearby there is every
> possibility that gd->relocaddr may overlap with framebuffer, also the code in
> reserve_trace, reserve_uboot doesn't have any intelligence or check that it is
> overlapping with framebuffer area or not.
>
> I think one thing that can probably be done here is to have a check that if
> passed framebuffer area falls within current relocaddr region, then update the
> relocaddr else don't touch relocaddr :
>
> if (ho->fb <= gd->relocaddr - ho->size)
> //It means framebuffer are is overlapping with current relocaddr so update
> relocaddr
> gd->relocaddr = ho->fb
> else
> //don't update gd->relocaddr since ho->fb is disjoint to gd->relocaddr
>
> Could you please share your opinion on this and if above logic suffice your
> case too ?
I don't think this line is needed at all, which is why this patch
removes it. What problem are you seeing?
Regards,
Simon
>
> Regards
> Devarsh
>
> > The SPL frame buffer needs to be considered pre-allocated. It makes no
> > sense to set relocaddr to this value. It will break all sorts of
> > things. E.g. qemu-x86_64 crashes with this.
> >
> >>
> >>
> >> Regards
> >> Devarsh
> >>
> >>> } else if (CONFIG_IS_ENABLED(VIDEO)) {
> >>> ulong addr;
> >>> int ret;
> >
> > Regards,
> > Simon
Re: [PATCH v2 2/4] rockchip: Add support to generate LZMA compressed U-boot binary
On Mon, 24 Jul 2023 at 21:51, Manoj Sai wrote: > > Add support for generating a LZMA-compressed U-boot binary with the > help of binman, if CONFIG_SPL_LZMA is selected. > > Signed-off-by: Manoj Sai > --- > arch/arm/dts/rockchip-u-boot.dtsi | 4 > 1 file changed, 4 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH v2 3/4] spl: fit: support for booting a GZIP-compressed U-boot binary
On Mon, 24 Jul 2023 at 21:51, Manoj Sai wrote: > > If GZIP Compression support is enabled, GZIP compressed U-Boot binary > will be at a specified RAM location which is defined at > CONFIG_SYS_LOAD_ADDR and will be assign it as the source address. > > gunzip function in spl_load_fit_image ,will decompress the GZIP > compressed U-Boot binary which is placed at > source address(CONFIG_SYS_LOAD_ADDR) to the default > CONFIG_SYS_TEXT_BASE location. > > spl_load_fit_image function will load the decompressed U-Boot > binary, which is placed at the CONFIG_SYS_TEXT_BASE location. > > Signed-off-by: Manoj Sai > Signed-off-by: Suniel Mahesh > --- > common/spl/spl_fit.c | 7 +-- > include/spl.h| 10 ++ > 2 files changed, 15 insertions(+), 2 deletions(-) > Reviewed-by: Simon Glass We could really use sandbox_spl test cases for these sorts of thing.
Re: [PATCH 1/1] efi_loader: simplify dp_fill()
On Wed, 26 Jul 2023 at 07:02, Heinrich Schuchardt
wrote:
>
> On 23.07.23 05:48, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 21 Jul 2023 at 00:34, Heinrich Schuchardt
> > wrote:
> >>
> >> Move the recursive dp_fill(dev->parent) call to a single location.
> >> Determine uclass_id only once.
> >>
> >> Signed-off-by: Heinrich Schuchardt
> >> ---
> >> lib/efi_loader/efi_device_path.c | 45 +---
> >> 1 file changed, 18 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/lib/efi_loader/efi_device_path.c
> >> b/lib/efi_loader/efi_device_path.c
> >> index 5b050fa17c..ed7214f3a3 100644
> >> --- a/lib/efi_loader/efi_device_path.c
> >> +++ b/lib/efi_loader/efi_device_path.c
> >> @@ -548,14 +548,19 @@ __maybe_unused static unsigned int dp_size(struct
> >> udevice *dev)
> >>*/
> >> __maybe_unused static void *dp_fill(void *buf, struct udevice *dev)
> >> {
> >> + enum uclass_id uclass_id;
> >> +
> >> if (!dev || !dev->driver)
> >> return buf;
> >>
> >> - switch (device_get_uclass_id(dev)) {
> >> + uclass_id = device_get_uclass_id(dev);
> >> + if (uclass_id != UCLASS_ROOT)
> >
> > Can we fix this one now? We should use EFI_MEDIA here I think?
>
> The function dp_fill() is used to create an EFI device path for any DM
> device.
>
> Given a device we recursively create a device path with nodes for every
> device in the dm tree up to the root node. We must stop the recursion at
> the root node.
>
> >
> >> + buf = dp_fill(buf, dev->parent);
> >> +
> >> + switch (uclass_id) {
> >> #ifdef CONFIG_NETDEVICES
> >> case UCLASS_ETH: {
> >> - struct efi_device_path_mac_addr *dp =
> >> - dp_fill(buf, dev->parent);
> >
> > So how does the parent part get added? I am missing something
> > here...or it was it never needed??
>
> dp_fill() is a recursive function as explained above.
OK I see
Reviewed-by: Simon Glass
Regards,
SImon
Re: [PATCH v2 04/12] firmware: scmi: framework for installing additional protocols
On Wed, 26 Jul 2023 at 02:39, AKASHI Takahiro wrote: > > This framework allows SCMI protocols to be installed and bound to the agent > so that the agent can manage and utilize them later. > > Signed-off-by: AKASHI Takahiro > --- > v2 > * check for availability of protocols > --- > drivers/firmware/scmi/scmi_agent-uclass.c | 100 +- > include/scmi_agent-uclass.h | 15 +++- > include/scmi_agent.h | 14 +++ > 3 files changed, 125 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 09/12] test: dm: add SCMI base protocol test
\On Wed, 26 Jul 2023 at 02:39, AKASHI Takahiro wrote: > > Added is a new unit test for SCMI base protocol, which will exercise all > the commands provided by the protocol, except SCMI_BASE_NOTIFY_ERRORS. > $ ut dm scmi_base > It is assumed that test.dtb is used as sandbox's device tree. > > Signed-off-by: AKASHI Takahiro > --- > v2 > * use helper functions, removing direct uses of ops > --- > test/dm/scmi.c | 109 + > 1 file changed, 109 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH v2 4/4] spl: fit: support for booting a LZMA-compressed U-boot binary
On Mon, 24 Jul 2023 at 21:51, Manoj Sai wrote: > > If LZMA Compression support is enabled, LZMA compressed U-Boot > binary will be placed at a specified RAM location which is > defined at CONFIG_SYS_LOAD_ADDR and will be assigned as the > source address. > > image_decomp() function, will decompress the LZMA compressed > U-Boot binary which is placed at source address(CONFIG_SYS_LOAD_ADDR) > to the default CONFIG_SYS_TEXT_BASE location. > > spl_load_fit_image function will load the decompressed U-Boot > binary, which is placed at the CONFIG_SYS_TEXT_BASE location. > > Signed-off-by: Manoj Sai > Signed-off-by: Suniel Mahesh > --- > common/spl/spl_fit.c | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 1/4] rockchip: Add support to generate GZIP compressed U-boot binary
On Mon, 24 Jul 2023 at 21:51, Manoj Sai wrote: > > Add support for generating a GZIP-compressed U-boot binary with the > help of binman, if CONFIG_SPL_GZIP is selected. > > Signed-off-by: Manoj Sai > --- > arch/arm/dts/rockchip-u-boot.dtsi | 7 +++ > 1 file changed, 7 insertions(+) Reviewed-by: Simon Glass
Re: [RFC PATCH 03/10] remoteproc: add remoteproc virtio transport driver
Hi Tanmay,
On Tue, 25 Jul 2023 at 08:08, Tanmay Shah wrote:
>
> Remoteproc virtio is virtio transport layer driver for remoteproc.
> Implement virtio config ops used by actual virtio
> driver using remoteproc virtio device
>
> Ported from the Linux kernel version 6.4-rc2 (d848a4819d85),
> Introduced in kernel version v3.10 (ac8954a41393)
> file: drivers/remoteproc/remoteproc_virtio.c.
>
> Signed-off-by: Tanmay Shah
> ---
> MAINTAINERS | 6 +
> drivers/remoteproc/Kconfig| 11 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/rproc-uclass.c | 42 ++-
> drivers/remoteproc/rproc_virtio.c | 422 ++
> drivers/virtio/virtio_ring.c | 16 ++
> include/remoteproc.h | 66 +++--
> include/rproc_virtio.h| 29 ++
> include/virtio.h | 3 +
> include/virtio_ring.h | 17 ++
> 10 files changed, 593 insertions(+), 20 deletions(-)
> create mode 100644 drivers/remoteproc/rproc_virtio.c
> create mode 100644 include/rproc_virtio.h
Can you split this patch up a bit? It seems to add a new method and
lots of other stuff.
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 87991cccdd..c4a32a0956 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1319,6 +1319,12 @@ S: Maintained
> T: git https://source.denx.de/u-boot/custodians/u-boot-nand-flash.git
> F: drivers/mtd/nand/raw/
>
> +REMOTEPROC
> +M: Tanmay Shah
> +S: Maintained
> +F: drivers/remoteproc/rproc_virtio.c
> +F: include/rproc_virtio.h
> +
> RISC-V
> M: Rick Chen
> M: Leo
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 27e4a60ff5..b758c248e4 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -102,4 +102,15 @@ config REMOTEPROC_TI_IPU
> help
> Say 'y' here to add support for TI' K3 remoteproc driver.
>
> +config REMOTEPROC_VIRTIO
> + bool "Support remoteproc virtio devices"
> + select REMOTEPROC
> + select VIRTIO
> + depends on DM
> + help
> + Say 'y' here to add support of remoteproc virtio devices.
> + rproc_virtio is virtio transport layer driver. The transport
> + drivers provide a set of ops for the real virtio device
> + driver to call.
> +
> endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index fbe9c172bc..61fdb87efb 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_REMOTEPROC_TI_K3_R5F) += ti_k3_r5f_rproc.o
> obj-$(CONFIG_REMOTEPROC_TI_POWER) += ti_power_proc.o
> obj-$(CONFIG_REMOTEPROC_TI_PRU) += pru_rproc.o
> obj-$(CONFIG_REMOTEPROC_TI_IPU) += ipu_rproc.o
> +obj-$(CONFIG_REMOTEPROC_VIRTIO) += rproc_virtio.o
> diff --git a/drivers/remoteproc/rproc-uclass.c
> b/drivers/remoteproc/rproc-uclass.c
> index d697639cdd..3aebaf6187 100644
> --- a/drivers/remoteproc/rproc-uclass.c
> +++ b/drivers/remoteproc/rproc-uclass.c
> @@ -14,6 +14,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -279,6 +280,33 @@ static int rproc_config_pagetable(struct udevice *dev,
> unsigned int virt,
> return 0;
> }
>
> +/**
> + * rproc_find_res_by_name() - After parsing the resource table add the
> mappings
> + * @dev: device we finished probing
> + * @name: name of rproc_mem_entry resource
> + *
> + * Return: If failed NULL, else first carveout entry with matching name.
> + */
> +struct rproc_mem_entry *rproc_find_res_by_name(struct udevice *dev,
> + const char *name)
> +{
> + struct rproc *rproc = rproc_get_cfg(dev);
> + struct rproc_mem_entry *mapping = NULL;
> + int ret;
> +
> + if (!rproc)
> + return NULL;
> +
> + list_for_each_entry(mapping, &rproc->mappings.node, node) {
> + ret = strcmp(mapping->name, name);
> + if (!ret)
Do we need ret?
> + return mapping;
> + }
> +
> + debug("%s: %s carveout not found\n", dev->name, name);
We typically put a blank line before the final return
> + return NULL;
> +}
> +
> UCLASS_DRIVER(rproc) = {
> .id = UCLASS_REMOTEPROC,
> .name = "remoteproc",
> @@ -688,6 +716,7 @@ static int alloc_vring(struct udevice *dev, struct
> fw_rsc_vdev *rsc, int i)
> static int handle_vdev(struct udevice *dev, struct fw_rsc_vdev *rsc,
>int offset, int avail)
> {
> + struct rproc *rproc;
> int i, ret;
> void *pa;
>
> @@ -720,7 +749,18 @@ static int handle_vdev(struct udevice *dev, struct
> fw_rsc_vdev *rsc,
> }
>
> /*
> -* allocate the vrings
> +* If virtio device creation is supported, then prefer to get vrings
> +* during find_vq op
> +*/
> + rproc = rproc_get_cfg(dev);
> +
> + if (rproc && r
Re: [PATCH v2 12/12] test: dm: add scmi command test
On Wed, 26 Jul 2023 at 02:40, AKASHI Takahiro wrote: > > In this test, "scmi" command is tested against different sub-commands. > Please note that scmi command is for debug purpose and is not intended > in production system. > > Signed-off-by: AKASHI Takahiro > Reviewed-by: Simon Glass > --- > v2 > * use helper functions, removing direct uses of ops > --- > test/dm/scmi.c | 59 +++--- > 1 file changed, 56 insertions(+), 3 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 4/5] mmc: Use regulator_set_enable_if_allowed
On Wed, 19 Jul 2023 at 15:21, Jonas Karlman wrote:
>
> With the commit 4fcba5d556b4 ("regulator: implement basic reference
> counter") the return value of regulator_set_enable may be EALREADY or
> EBUSY for fixed/gpio regulators.
>
> Change to use the more relaxed regulator_set_enable_if_allowed to
> continue if regulator already was enabled or disabled.
>
> Signed-off-by: Jonas Karlman
> ---
> drivers/mmc/mmc.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass
Tested-by: Simon Glass # rockpro64-rk3399
Re: [PATCH 18/18] riscv: qemu: Enable usb keyboard as an input device
On Tue, 25 Jul 2023 at 01:05, Rick Chen wrote: > > > From: U-Boot On Behalf Of Bin Meng > > Sent: Sunday, July 23, 2023 12:41 PM > > To: Simon Glass ; u-boot@lists.denx.de > > Cc: Bin Meng ; Heinrich Schuchardt > > Subject: [PATCH 18/18] riscv: qemu: Enable usb keyboard as an input device > > > > This brings PCI xHCI support to QEMU RISC-V and uses a usb keyboard as one > > of the input devices. > > > > Signed-off-by: Bin Meng > > > > --- > > > > board/emulation/qemu-riscv/Kconfig | 5 + > > board/emulation/qemu-riscv/qemu-riscv.c | 5 + > > doc/board/emulation/qemu-riscv.rst | 5 + > > include/configs/qemu-riscv.h| 2 +- > > 4 files changed, 16 insertions(+), 1 deletion(-) > > Reviewed-by: Rick Chen Reviewed-by: Simon Glass
Re: [PATCH 2/5] iot2050: Use binman in signing script
On Wed, 26 Jul 2023 at 09:21, Jan Kiszka wrote: > > From: Jan Kiszka > > The underlying issue was fixed in the meantime. Also signing the U-Boot > proper fit image now works. Just supporting custom cert templates > remains a todo. > > Signed-off-by: Jan Kiszka > --- > CC: Simon Glass > --- > tools/iot2050-sign-fw.sh | 9 ++--- > 1 file changed, 2 insertions(+), 7 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 11/12] doc: cmd: add documentation for scmi
On Wed, 26 Jul 2023 at 02:40, AKASHI Takahiro wrote: > > This is a help text for scmi command. > > Signed-off-by: AKASHI Takahiro > --- > v2 > * add more descriptions about SCMI > --- > doc/usage/cmd/scmi.rst | 126 + > 1 file changed, 126 insertions(+) > create mode 100644 doc/usage/cmd/scmi.rst Reviewed-by: Simon Glass
Re: [PATCH v2 10/12] cmd: add scmi command for SCMI firmware
On Wed, 26 Jul 2023 at 02:40, AKASHI Takahiro wrote: > > This command, "scmi", may provide a command line interface to various SCMI > protocols. It supports at least initially SCMI base protocol and is > intended mainly for debug purpose. > > Signed-off-by: AKASHI Takahiro > --- > v2 > * remove sub command category, 'scmi base', for simplicity > --- > cmd/Kconfig | 9 ++ > cmd/Makefile | 1 + > cmd/scmi.c | 334 +++ > 3 files changed, 344 insertions(+) > create mode 100644 cmd/scmi.c > Reviewed-by: Simon Glass
Re: [PATCH v2 05/12] firmware: scmi: install base protocol to SCMI agent
Hi,
On Wed, 26 Jul 2023 at 02:39, AKASHI Takahiro
wrote:
>
> SCMI base protocol is mandatory, and once SCMI node is found in a device
> tree, the protocol handle (udevice) is unconditionally installed to
> the agent. Then basic information will be retrieved from SCMI server via
> the protocol and saved into the agent instance's local storage.
>
> Signed-off-by: AKASHI Takahiro
> ---
> v2
> * use helper functions, removing direct uses of ops
> ---
> drivers/firmware/scmi/scmi_agent-uclass.c | 116 ++
> include/scmi_agent-uclass.h | 70 -
> 2 files changed, 184 insertions(+), 2 deletions(-)
>
Reviewed-by: Simon Glass
> diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c
> b/drivers/firmware/scmi/scmi_agent-uclass.c
> index 2244fcf487e8..279c2c218913 100644
> --- a/drivers/firmware/scmi/scmi_agent-uclass.c
> +++ b/drivers/firmware/scmi/scmi_agent-uclass.c
> @@ -57,6 +57,9 @@ struct udevice *scmi_get_protocol(struct udevice *dev,
> }
>
> switch (id) {
> + case SCMI_PROTOCOL_ID_BASE:
> + proto = priv->base_dev;
> + break;
> case SCMI_PROTOCOL_ID_CLOCK:
> proto = priv->clock_dev;
> break;
> @@ -101,6 +104,9 @@ static int scmi_add_protocol(struct udevice *dev,
> }
>
> switch (proto_id) {
> + case SCMI_PROTOCOL_ID_BASE:
> + priv->base_dev = proto;
> + break;
> case SCMI_PROTOCOL_ID_CLOCK:
> priv->clock_dev = proto;
> break;
> @@ -229,6 +235,83 @@ int devm_scmi_process_msg(struct udevice *dev, struct
> scmi_msg *msg)
> return scmi_process_msg(parent, priv->channel, msg);
> }
>
> +/**
> + * scmi_fill_base_info - get base information about SCMI server
> + * @agent: SCMI agent device
> + * @dev: SCMI protocol device
> + *
> + * By using Base protocol commands, collect the base information
> + * about SCMI server.
> + *
> + * Return: 0 on success, error code otherwise
> + */
> +static int scmi_fill_base_info(struct udevice *agent, struct udevice *dev)
> +{
> + struct scmi_agent_priv *priv = dev_get_uclass_plat(agent);
> + int ret;
> +
> + ret = scmi_base_protocol_version(dev, &priv->version);
> + if (ret) {
> + dev_err(dev, "protocol_version() failed (%d)\n", ret);
> + return ret;
> + }
> + /* check for required version */
> + if (priv->version < SCMI_BASE_PROTOCOL_VERSION) {
> + dev_err(dev, "base protocol version (%d) lower than
> expected\n",
> + priv->version);
> + return -EPROTO;
> + }
> +
> + ret = scmi_base_protocol_attrs(dev, &priv->num_agents,
> + &priv->num_protocols);
> + if (ret) {
> + dev_err(dev, "protocol_attrs() failed (%d)\n", ret);
> + return ret;
> + }
> + ret = scmi_base_discover_vendor(dev, priv->vendor);
> + if (ret) {
> + dev_err(dev, "base_discover_vendor() failed (%d)\n", ret);
> + return ret;
> + }
> + ret = scmi_base_discover_sub_vendor(dev, priv->sub_vendor);
> + if (ret) {
> + if (ret != -EOPNOTSUPP) {
> + dev_err(dev, "base_discover_sub_vendor() failed
> (%d)\n",
> + ret);
> + return ret;
> + }
> + strcpy(priv->sub_vendor, "NA");
> + }
> + ret = scmi_base_discover_impl_version(dev, &priv->impl_version);
> + if (ret) {
> + dev_err(dev, "base_discover_impl_version() failed (%d)\n",
> + ret);
> + return ret;
> + }
> +
> + priv->agent_id = 0x; /* to avoid false claim */
> + ret = scmi_base_discover_agent(dev, 0x,
> + &priv->agent_id, priv->agent_name);
> + if (ret) {
> + if (ret != -EOPNOTSUPP) {
> + dev_err(dev,
> + "base_discover_agent() failed for myself
> (%d)\n",
> + ret);
> + return ret;
> + }
> + priv->agent_name[0] = '\0';
> + }
> +
> + ret = scmi_base_discover_list_protocols(dev, &priv->protocols);
> + if (ret != priv->num_protocols) {
> + dev_err(dev, "base_discover_list_protocols() failed (%d)\n",
> + ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> /*
> * SCMI agent devices binds devices of various uclasses depeding on
> * the FDT description. scmi_bind_protocol() is a generic bind sequence
> @@ -243,6 +326,39 @@ static int scmi_bind_protocols(struct udevice *dev)
> struct driver *drv;
> struct udevice *proto;
>
> + if (!uclass_get_device(UCLASS_SCMI_A
Re: [PATCH 1/1] fat: correct sign for deletion mark
On Wed, 26 Jul 2023 at 02:33, Heinrich Schuchardt
wrote:
>
> The FAT file systems uses character '\xe5' to mark a deleted directory
> entry. If a file name starts with this character, it is substituted by
> '\x05' in the directory entry.
>
> While (signed char)'\xe5' is a negative number 0xe5 is a positive integer
> number. We therefore have define a constant DELETED_MARK which matches the
> signedness of the characters in the directory entry.
>
> Correct a comparison where we used the constant 0xe5 with the wrong sign.
> Use the constant aRING instead of 0x05 like in the rest of the code.
>
> Reported-by: Dan Carpenter
> Fixes: 57b745e2387a ("fs: fat: call set_name() only once")
> Fixes: 28cef9ca2e86 ("fs: fat: create correct short names")
> Signed-off-by: Heinrich Schuchardt
> ---
> fs/fat/fat_write.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass
Could we have a test for this?
Re: [RFC PATCH 1/3] scripts: kconfig: Add config fragment support in board/../
Hi Tom, On Wed, 19 Jul 2023 at 07:34, Tom Rini wrote: > > On Tue, Jul 18, 2023 at 07:07:58PM -0600, Simon Glass wrote: > > Hi, > > > > On Sun, 16 Jul 2023 at 09:12, Tom Rini wrote: > > > > > > On Sat, Jul 15, 2023 at 05:40:35PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Thu, 13 Jul 2023 at 16:54, Tom Rini wrote: > > > > > > > > > > On Wed, Jul 12, 2023 at 08:00:28AM -0600, Simon Glass wrote: > > > > > > Hi Jason, > > > > > > > > > > > > On Tue, 11 Jul 2023 at 16:29, Jason Kacines > > > > > > wrote: > > > > > > > > > > > > > > Add support to config fragments (.config) located in the /board > > > > > > > directory. This will allow only base defconfigs to live in > > > > > > > /configs and > > > > > > > > > > > > Does this mean defconfigs? > > > > > > > > > > This looks like it would cover defconfig files too, but the initial > > > > > motivation is config fragments. See > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20230606071850.270001-5-clamo...@gmail.com/ > > > > > for another example. > > > > > > > > > > > > all fragments to live in their respective device directory in > > > > > > > /board/.. > > > > > > > > > > > > Why do we want this? The patch should have a motivation. > > > > > > > > > > I've asked a few people to look in to this because we have a lot of > > > > > cases today of N _defconfig files where we could really instead have 1 > > > > > _defconfig file and N config fragment files. But I do not want them > > > > > living in the top level configs directory as that will get even more > > > > > unmanageable. > > > > > > > > OK I see, thank you. The patch still needs this motivation though. > > > > > > So you're saying you want the message re-worded? > > > > Yes, to explain why. > > I think the message is fine as it clearly says what it does. > > > Could we also get some docs in doc/build/gcc.rst or similar? > > No, I don't think that makes sense yet. And looking at that doc, we > should split that up in to compiler specifics and then a general build > doc. Using fragments belongs in the board docs which use fragments (as Indeed. +Heinrich Schuchardt perhaps? > is done in the series which have boards using fragments) and as a > general "do this to make developing your board easier" that should come > later once there's more agreement and understanding of what we can and > should do with fragments, rather than meta-options in Kconfig. Yes it is that understanding that I am missing. > > > > > > What's not in this patch (and not an ask at this point) is figuring > > > > > out > > > > > how buildman could handle "foo_defconfig bar.config" as the required > > > > > config target. > > > > > > > > Indeed. Also, should they appear in the boards.cfg list? > > > > > > I doubt it? I'm not sure yet how we address getting buildman to know > > > about valid additional combinations. Take the example of something like: > > > som_vendor_carrier_defconfig + som_vendor_imx7_som.config + > > > emmc_boot_instead.config + customer_production_tweaks.config > > > > > > How would you want buildman to know about that? Does it even really need > > > to, on the other hand? And that's not I think an uncommon example, it's > > > just splitting colibri_imx7_emmc_defconfig in to how it would be used by > > > someone taking that carrier+som to production, with their own > > > touchscreen and a few other tweaks in the dtb that needs to be passed to > > > linux. Or the mnt reform with whatever SOM/COM you happen to have for > > > it. > > > > Well firstly we should only worry about things that are in-tree. > > Well, since I'm not letting people bring in fragments until it won't > make the configs directory even more unmanageable, we have a problem. > The problem which this patch solves. And the example I gave above is > in-tree, except for the final step of "now make this my product", which > when it's a matter of a new device tree and config, is fine for most > cases. My objection is really that this changes / adds behaviour for which there is no mention in the docs. I didn't even know about this stuff. > > > The thing is, if we don't validate that the configs at least build, > > then someone could change a config (anywhere in Kconfig or in a 'base' > > defconfig) and break the build for these 'add-on' configs. Also if > > I'm not worried about this at the start. If people start trying to > enable unique drivers only in fragments, we have a problem. But based on > all of the proposed uses so far, we shouldn't see unique settings there > that we need to have compile tested all the time. OK > > > there is no record of what fragments can be built with what, it could > > get awfully confusing. > > Exactly why I want these fragments to be able to live in the board > directory rather than the top level configs directory. Honestly, this > also opens up the possibility of moving the defconfig files from > configs/ to the board directory and I think that would be really good. Yes it w
Re: [PATCH v2 03/12] firmware: scmi: move scmi_bind_protocols() backward
On Wed, 26 Jul 2023 at 02:38, AKASHI Takahiro wrote: > > Move the location of scmi_bind_protocols() backward for changes > in later patches. > There is no change in functionality. > > Signed-off-by: AKASHI Takahiro > --- > drivers/firmware/scmi/scmi_agent-uclass.c | 118 +++--- > 1 file changed, 59 insertions(+), 59 deletions(-) Reviewed-by: Simon Glass
Re: [RFC PATCH 04/10] drivers: add RPMsg framework
Hi Tanmay,
On Tue, 25 Jul 2023 at 08:08, Tanmay Shah wrote:
>
> RPMsg framework is used to communicate to remote processor
> using rpmsg protocol.
>
> This framework is ported from the Linux kernel
> directory: drivers/rpmsg/
> kernel version: 6.4-rc2 (d848a4819d85)
>
> Signed-off-by: Tanmay Shah
> ---
> MAINTAINERS| 7 +
> arch/sandbox/dts/test.dts | 8 +
> drivers/Kconfig| 2 +
> drivers/Makefile | 1 +
> drivers/rpmsg/Kconfig | 31 +++
> drivers/rpmsg/Makefile | 10 +
> drivers/rpmsg/rpmsg-uclass.c | 156
> drivers/rpmsg/rpmsg_internal.h | 52
> drivers/rpmsg/sandbox_test_rpmsg.c | 88 +++
> drivers/rpmsg/virtio_rpmsg_bus.c | 384 +
> drivers/virtio/virtio-uclass.c | 1 +
> include/dm/uclass-id.h | 1 +
> include/rpmsg.h| 140 +++
> include/rproc_virtio.h | 8 +-
> include/virtio.h | 4 +-
> include/virtio_ring.h | 15 ++
> test/dm/Makefile | 1 +
> test/dm/rpmsg.c| 41 +++
> 18 files changed, 947 insertions(+), 3 deletions(-)
> create mode 100644 drivers/rpmsg/Kconfig
> create mode 100644 drivers/rpmsg/Makefile
> create mode 100644 drivers/rpmsg/rpmsg-uclass.c
> create mode 100644 drivers/rpmsg/rpmsg_internal.h
> create mode 100644 drivers/rpmsg/sandbox_test_rpmsg.c
> create mode 100644 drivers/rpmsg/virtio_rpmsg_bus.c
> create mode 100644 include/rpmsg.h
> create mode 100644 test/dm/rpmsg.c
Reviewed-by: Simon Glass
Again there is a lot going on in this patch and it could use a split.
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c4a32a0956..876a7fdbdf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1365,6 +1365,13 @@ F: drivers/usb/gadget/f_rockusb.c
> F: cmd/rockusb.c
> F: doc/README.rockusb
>
> +RPMSG
> +M: Tanmay Shah
> +S: Maintained
> +F: drivers/rpmsg/*
> +F: include/rpmsg.h
> +F: test/dm/rpmsg.c
> +
> SANDBOX
> M: Simon Glass
> S: Maintained
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index b5509eee8c..fca1a591fb 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -1247,6 +1247,14 @@
> compatible = "sandbox,sandbox-rng";
> };
>
> + rpmsg_1: rpmsg@1 {
> + compatible = "sandbox,test-rpmsg";
> + };
> +
> + rpmsg_2: rpmsg@2 {
> + compatible = "sandbox,test-rpmsg";
> + };
> +
> rproc_1: rproc@1 {
> compatible = "sandbox,test-processor";
> remoteproc-name = "remoteproc-test-dev1";
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index a25f6ae02f..69700f1f83 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -112,6 +112,8 @@ source "drivers/reset/Kconfig"
>
> source "drivers/rng/Kconfig"
>
> +source "drivers/rpmsg/Kconfig"
> +
> source "drivers/rtc/Kconfig"
>
> source "drivers/scsi/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 3bc6d279d7..68e8d8b065 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -109,6 +109,7 @@ obj-y += mfd/
> obj-y += mtd/
> obj-y += pwm/
> obj-y += reset/
> +obj-y += rpmsg/
> obj-y += input/
> obj-y += iommu/
> # SOC specific infrastructure drivers.
> diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
> new file mode 100644
> index 00..4efb8dfcd7
> --- /dev/null
> +++ b/drivers/rpmsg/Kconfig
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (C) 2023, Advanced Micro devices, Inc.
> +
> +menu "RPMsg drivers"
> +
> +# RPMsg gets selected by drivers as needed
> +# All users should depend on DM
> +config RPMSG
> + bool
> + depends on DM
> +
> +config VIRTIO_RPMSG_BUS
> + bool "virtio rpmsg bus"
> + depends on DM
> + select RPMSG
> + select REMOTEPROC_VIRTIO
> + help
> + Say 'y' here to enable virtio based RPMsg. RPMsg allows
> + U-Boot drivers to communicate with remote processors.
Please expand abbreviations in Kconfig
> +
> +config RPMSG_SANDBOX
> + bool "RPMsg driver for sandbox platform"
> + depends on DM
> + select RPMSG
> + depends on SANDBOX
> + help
> + Say 'y' here to add sandbox driver for RPMsg framework used
> + for dummy communication with remote processor on sandbox platform
> +
> +endmenu
> diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile
> new file mode 100644
> index 00..21611725ea
> --- /dev/null
> +++ b/drivers/rpmsg/Makefile
> @@ -0,0 +1,10 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (C) 2023, Advanced Micro Devices, Inc.
> +
> +obj-$(CONFIG_RPMSG) += rpmsg-uclass.o
> +
> +obj-$(CONFIG_RPMSG_SANDBOX) += sandbox_test_rpmsg.o
> +
> +# virtio driver for rpmsg
> +obj-$(CONFIG_VIRTIO_RPMSG_B
Re: [RFC] efi_driver: fix a parent issue in efi-created block devices
Hi,
On Wed, 19 Jul 2023 at 20:56, AKASHI Takahiro
wrote:
>
> On Wed, Jul 19, 2023 at 07:29:57PM -0600, Simon Glass wrote:
> > Hi,
> >
> > On Wed, 19 Jul 2023 at 18:14, AKASHI Takahiro
> > wrote:
> > >
> > > On Wed, Jul 19, 2023 at 03:15:10PM +0200, Heinrich Schuchardt wrote:
> > > > On 19.07.23 15:04, Simon Glass wrote:
> > > > > Hi,
> > > > >
> > > > > On Tue, 18 Jul 2023 at 19:54, AKASHI Takahiro
> > > > > wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Tue, Jul 18, 2023 at 07:08:45PM -0600, Simon Glass wrote:
> > > > > > > Hi AKASHI,
> > > > > > >
> > > > > > > On Tue, 18 Jul 2023 at 18:22, AKASHI Takahiro
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > An EFI application may create an EFI block device
> > > > > > > > (EFI_BLOCK_IO_PROTOCOL) in
> > > > > > > > EFI world, which in turn generates a corresponding U-Boot block
> > > > > > > > device based on
> > > > > > > > U-Boot's Driver Model.
> > > > > > > > The latter device, however, doesn't work as U-Boot proper block
> > > > > > > > device
> > > > > > > > due to an issue in efi_driver's implementation. We saw
> > > > > > > > discussions in the past,
> > > > > > > > most recently in [1].
> > > > > > > >
> > > > > > > >[1]
> > > > > > > > https://lists.denx.de/pipermail/u-boot/2023-July/522565.html
> > > > > > > >
> > > > > > > > This RFC patch tries to address (part of) the issue.
> > > > > > > > If it is considered acceptable, I will create a formal patch.
> > > > > > > >
> > > > > > > > Withtout this patch,
> > > > > > > > ===8<===
> > > > > > > > => env set efi_selftest 'block device'
> > > > > > > > => bootefi selftest
> > > > > > > > ...
> > > > > > > >
> > > > > > > > Summary: 0 failures
> > > > > > > >
> > > > > > > > => dm tree
> > > > > > > > Class Index Probed DriverName
> > > > > > > > ---
> > > > > > > > root 0 [ + ] root_driver root_driver
> > > > > > > > ...
> > > > > > > > bootmeth 7 [ ] vbe_simple| `--
> > > > > > > > vbe_simple
> > > > > > > > blk 0 [ + ] efi_blk `-- efiblk#0
> > > > > > > > partition 0 [ + ] blk_partition `--
> > > > > > > > efiblk#0:1
> > > > > > > > => ls efiloader 0:1
> > > > > > > > ** Bad device specification efiloader 0 **
> > > > > > > > Couldn't find partition efiloader 0:1
> > > > > > > > ===>8===
> > > > > > > >
> > > > > > > > With this patch applied, efiblk#0(:1) now gets accessible.
> > > > > > > >
> > > > > > > > ===8<===
> > > > > > > > => env set efi_selftest 'block device'
> > > > > > > > => bootefi selftest
> > > > > > > > ...
> > > > > > > >
> > > > > > > > Summary: 0 failures
> > > > > > > >
> > > > > > > > => dm tree
> > > > > > > > Class Index Probed DriverName
> > > > > > > > ---
> > > > > > > > root 0 [ + ] root_driver root_driver
> > > > > > > > ...
> > > > > > > > bootmeth 7 [ ] vbe_simple| `--
> > > > > > > > vbe_simple
> > > > > > > > efi 0 [ + ] EFI block driver `--
> > > > > > > > /VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)
> > > > > > > > blk 0 [ + ] efi_blk `--
> > > > > > > > efiblk#0
> > > > > > > > partition 0 [ + ] blk_partition `--
> > > > > > > > efiblk#0:1
> > > > > > > > => ls efiloader 0:1
> > > > > > > > 13 hello.txt
> > > > > > > > 7 u-boot.txt
> > > > > > > >
> > > > > > > > 2 file(s), 0 dir(s)
> > > > > > > > ===>8===
> > > > > > > >
> > > > > > > > Signed-off-by: AKASHI Takahiro
> > > > > > > > ---
> > > > > > > > include/efi_driver.h | 2 +-
> > > > > > > > lib/efi_driver/efi_block_device.c| 17
> > > > > > > > -
> > > > > > > > lib/efi_driver/efi_uclass.c | 8 +++-
> > > > > > > > lib/efi_selftest/efi_selftest_block_device.c | 2 ++
> > > > > > > > 4 files changed, 22 insertions(+), 7 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/include/efi_driver.h b/include/efi_driver.h
> > > > > > > > index 63a95e4cf800..ed66796f3519 100644
> > > > > > > > --- a/include/efi_driver.h
> > > > > > > > +++ b/include/efi_driver.h
> > > > > > > > @@ -42,7 +42,7 @@ struct efi_driver_ops {
> > > > > > > > const efi_guid_t *child_protocol;
> > > > > > > > efi_status_t (*init)(struct
> > > > > > > > efi_driver_binding_extended_protocol *this);
> > > > > > > > efi_status_t (*bind)(struct
> > > > > > > > efi_driver_binding_extended_protocol *this,
> > > > > > > > -efi_handle_t handle, void
> > > > > > > > *interface);
> > > > > > > > +efi_handle_t handle, void
> > > > > > > > *interface, char *name);
> > > > > > > > };
> > > > > > > >
> > > > > > > > #endi
Re: [RFC PATCH 06/10] cmd: add rpmsg framework commands
Hi Tanmay, On Tue, 25 Jul 2023 at 08:08, Tanmay Shah wrote: > > This patch introduces commands to use rpmsg framework > > rpmsg init > - Initialize rpmsg framework for remote core > > rpmsg debug_data <"tx" / "rx"> > - debug tx or rx virtqueues. This command simply uses > virtqueue_dump_data and prints virtqueue for rpmsg device > mapped to > > Signed-off-by: Tanmay Shah > --- > MAINTAINERS | 1 + > cmd/Kconfig | 6 + > cmd/Makefile| 1 + > cmd/rpmsg.c | 61 + > include/rpmsg.h | 5 > 5 files changed, 74 insertions(+) > create mode 100644 cmd/rpmsg.c > Please add doc/usage/cmd file as well as a test for your command in test/dm/rpmsg.c You can see test/dm/acpi.c for an example. Regards, Simon
Re: [RFC PATCH 10/10] configs: sandbox: enable rpmsg
On Tue, 25 Jul 2023 at 08:08, Tanmay Shah wrote: > > Enable configs required to test rpmsg on sandbox build. > CMD_MISC is enabled to probe rpmsg_sample_client driver. > > Test procedure: > - boot sandbox build > - command to run rpmsg tests: ut dm rpmsg > - command to run sample driver: misc list > > Signed-off-by: Tanmay Shah > --- > configs/sandbox_defconfig | 4 > 1 file changed, 4 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH] pci: Fix device_find_first_child() return value handling
Hi Marek,
On Mon, 17 Jul 2023 at 11:03, Marek Vasut wrote:
>
> On 7/17/23 09:42, Michal Suchánek wrote:
> > Hello,
> >
> > On Sun, Jul 16, 2023 at 05:53:24PM +0200, Marek Vasut wrote:
> >> This function only ever returns 0, but may not assign the second
> >> parameter. Same thing for device_find_next_child(). Do not assign
> >> ret to stop proliferation of this misuse.
> >>
> >> Reported-by: Jonas Karlman
> >> Signed-off-by: Marek Vasut
> >> ---
> >> Cc: "Pali Rohár"
> >> Cc: Bin Meng
> >> Cc: Marek Vasut
> >> Cc: Michal Suchanek
> >> Cc: Simon Glass
> >> ---
> >> drivers/pci/pci-uclass.c | 6 +++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> >> index 8d27e40338c..6421eda7721 100644
> >> --- a/drivers/pci/pci-uclass.c
> >> +++ b/drivers/pci/pci-uclass.c
> >> @@ -545,9 +545,9 @@ int pci_auto_config_devices(struct udevice *bus)
> >> sub_bus = dev_seq(bus);
> >> debug("%s: start\n", __func__);
> >> pciauto_config_init(hose);
> >> -for (ret = device_find_first_child(bus, &dev);
> >> - !ret && dev;
> >> - ret = device_find_next_child(&dev)) {
> >> +for (device_find_first_child(bus, &dev);
> >> + dev;
> >> + device_find_next_child(&dev)) {
Reviewed-by: Simon Glass
> >
> > Sounds like you will need to remove the declaration of the now unused ret
> > variable as well.
Yes, please remove the 'ret' at the top of the function.
> >
> > More generally, what is the overall vision for these functions returning
> > always zero?
> >
> > Should the return value be kept in case the underlying implementation
> > changes and errors can happen in the future, and consequently checked?
> >
> > Should the return value be removed when meaningless making these
> > useless assignments and checks an error?
> >
> > I already elimimnated a return value where using it lead to incorrect
> > behavior but here using it or not is equally correct with the current
> > implementation.
>
> Probably a question for Simon, really. Personally I would be tempted to
> switch the function to return void.
So long as the function has its meaning documented, I think it is OK.
As a separate patch, I am OK with changing
device_find_first/next_child() to void, or alternatively having them
return 0 on success and -ENODEV if nothing was found.
Regards,
Simon
Re: [RFC PATCH 07/10] remoteproc: add attach/detach commands
On Tue, 25 Jul 2023 at 08:08, Tanmay Shah wrote: > > Current remoteproc framework provides commands to start/stop > remote processor. However, If remote processor is already running > before U-Boot then there is no way to connect to remote processor. > > Implements attach/detach commands to connect to > already running remote processor. During attach operation, > remoteproc framework gets resource table from remote processor > using platform specific callback and then creates vdev devices > and other resources accordingly. > > This approach is derived from the Linux kernel remoteproc framework. > > kernel version: 6.4-rc2 (d848a4819d85) > file: drivers/remoteproc/remoteproc_core > > Signed-off-by: Tanmay Shah > --- > cmd/remoteproc.c | 14 +- > drivers/remoteproc/rproc-uclass.c | 76 ++- > include/remoteproc.h | 31 + > 3 files changed, 119 insertions(+), 2 deletions(-) This seems to need an additional test. Regards, Simon
Re: [RFC PATCH 05/10] rpmsg: add sample client driver
Hi Tanmay,
On Tue, 25 Jul 2023 at 08:08, Tanmay Shah wrote:
>
> Add driver to demonstrate use of rpmsg framework.
>
> Signed-off-by: Tanmay Shah
> ---
> drivers/rpmsg/Kconfig | 8
> drivers/rpmsg/Makefile | 3 ++
> drivers/rpmsg/rpmsg_sample_client.c | 63 +
> 3 files changed, 74 insertions(+)
> create mode 100644 drivers/rpmsg/rpmsg_sample_client.c
Reviewed-by: Simon Glass
>
> diff --git a/drivers/rpmsg/Kconfig b/drivers/rpmsg/Kconfig
> index 4efb8dfcd7..808cd16275 100644
> --- a/drivers/rpmsg/Kconfig
> +++ b/drivers/rpmsg/Kconfig
> @@ -28,4 +28,12 @@ config RPMSG_SANDBOX
> Say 'y' here to add sandbox driver for RPMsg framework used
> for dummy communication with remote processor on sandbox platform
>
> +config RPMSG_SAMPLE_CLIENT
> + bool "Sample RPMsg client driver"
> + depends on DM
> + select RPMSG
> + help
> + Say 'y' here to enable driver that demonstrate use of RPMsg
> framework.
enable a driver
demonstrates
> + This driver uses RPMsg APIs to communicate with remote processor.
> +
> endmenu
> diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile
> index 21611725ea..53af0f7ea6 100644
> --- a/drivers/rpmsg/Makefile
> +++ b/drivers/rpmsg/Makefile
> @@ -6,5 +6,8 @@ obj-$(CONFIG_RPMSG) += rpmsg-uclass.o
>
> obj-$(CONFIG_RPMSG_SANDBOX) += sandbox_test_rpmsg.o
>
> +# Sample driver demonstrate how to use rpmsg framework
> +obj-$(CONFIG_RPMSG_SAMPLE_CLIENT) += rpmsg_sample_client.o
> +
> # virtio driver for rpmsg
> obj-$(CONFIG_VIRTIO_RPMSG_BUS) += virtio_rpmsg_bus.o
> diff --git a/drivers/rpmsg/rpmsg_sample_client.c
> b/drivers/rpmsg/rpmsg_sample_client.c
> new file mode 100644
> index 00..ae591dbdde
> --- /dev/null
> +++ b/drivers/rpmsg/rpmsg_sample_client.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2011 Texas Instruments, Inc.
> + * Copyright (C) 2011 Google, Inc.
> + * Copyright (C) 2023, Advanced Micro Devices, Inc.
> + *
> + * Sample client that shows use of rpmsg APIs
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
Do you need those three?
> +#include
> +
> +static int rpmsg_rx_callback(void *buf, int msg_len, u32 msgs_received)
> +{
> + printf("APU: rx: %s\n", (char *)buf);
> +
> + return 0;
> +}
> +
> +static int rpmsg_sample_client_probe(struct udevice *udev)
> +{
> + char data[80] = {0};
> + int i;
> +
> + /* Initialize rpmsg for core 0 */
> + rpmsg_init(0);
> +
> + /* wait for RPU to initialize vdev */
> + mdelay(20);
> +
> + /* assume rpmsg init is already done */
> + for (i = 1; i < 5; i++) {
> + sprintf(data, "rpmsg buf %d", i);
> + printf("APU: tx: %s\n", data);
> + rpmsg_send(0, data, strlen(data));
> +
> + /* 20ms delay before receiving the data */
> + mdelay(20);
> +
> + rpmsg_recv(0, rpmsg_rx_callback);
> + printf("\n");
> + }
> + return 0;
> +}
> +
> +U_BOOT_DRIVER(rpmsg_sample_client) = {
> + .name = "rpmsg-sample-client",
> + .id = UCLASS_MISC,
> + .probe = rpmsg_sample_client_probe,
> + .flags = DM_FLAG_PRE_RELOC,
> +};
> +
> +U_BOOT_DRVINFO(rpmsg_sample_client) = {
> + .name = "rpmsg-sample-client",
> +};
> --
> 2.25.1
>
REgards,
SImon
Re: [PATCH v5 23/46] pci: Allow the video BIOS to work in SPL with QEMU
Hi Bin,
On Mon, 17 Jul 2023 at 00:20, Bin Meng wrote:
>
> Hi Simon,
>
> On Sun, Jul 16, 2023 at 11:39 AM Simon Glass wrote:
> >
> > QEMU emulates two common machines (Q35 and i440fx) which use mapping to
> > determine whether RAM is present below 1MB. In order to copy the video
> > BIOS to c we need to flip this mapping over to RAM. This does not
> > happen automatically until SPL has finished running.
> >
> > Switch in RAM at these address so that the video BIOS can be loaded and
> > run.
> >
> > Create a generic qemu.h header to avoid having to #ifdef the header in
> > pci_rom.c
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > Changes in v5:
> > - Use the existing QEMU code instead
> >
> > arch/x86/cpu/qemu/qemu.c | 5 +++--
> > drivers/pci/pci_rom.c| 5 +
> > include/qemu.h | 14 ++
> > 3 files changed, 22 insertions(+), 2 deletions(-)
> > create mode 100644 include/qemu.h
> >
> > diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
> > index 274978c023b6..a84d2aceda3c 100644
> > --- a/arch/x86/cpu/qemu/qemu.c
> > +++ b/arch/x86/cpu/qemu/qemu.c
> > @@ -7,6 +7,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -48,7 +49,7 @@ static void enable_pm_ich9(void)
> > pci_write_config32(ICH9_PM, PMBA, CONFIG_ACPI_PM1_BASE | 1);
> > }
> >
> > -static void qemu_chipset_init(void)
> > +void qemu_x86_chipset_init(void)
> > {
> > u16 device, xbcs;
> > int pam, i;
> > @@ -119,7 +120,7 @@ int print_cpuinfo(void)
> >
> > int arch_early_init_r(void)
> > {
> > - qemu_chipset_init();
> > + qemu_x86_chipset_init();
> >
> > return 0;
> > }
> > diff --git a/drivers/pci/pci_rom.c b/drivers/pci/pci_rom.c
> > index f0dfe6314907..447957fb23aa 100644
> > --- a/drivers/pci/pci_rom.c
> > +++ b/drivers/pci/pci_rom.c
> > @@ -34,6 +34,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -185,6 +186,10 @@ static int pci_rom_load(struct pci_rom_header
> > *rom_header,
> > return -ENOMEM;
> > *allocedp = true;
> > #endif
> > + /* QEMU hacks */
> > + if (IS_ENABLED(CONFIG_X86) && IS_ENABLED(CONFIG_QEMU))
> > + qemu_x86_chipset_init();
> > +
>
> Maybe I did not say it clearly in the previous version. I think we
> should avoid polluting the generic pci_rom.c codes.
>
> First of all, these are not QEMU hacks, but are required operations
> against a specific Intel chipset. QEMU just emulates the real hardware
> behavior.
>
> Second, we should update x86 SPL codes to call qemu_chipset_init() for
> QEMU x86_64.
OK I understand now. I sent a new series that includes a new patch\.
Regards,
Simon
Re: [PATCH 15/18] riscv: qemu: Enable PRE_CONSOLE_BUFFER
On Tue, 25 Jul 2023 at 01:21, Rick Chen wrote: > > > From: U-Boot On Behalf Of Bin Meng > > Sent: Sunday, July 23, 2023 12:41 PM > > To: Simon Glass ; u-boot@lists.denx.de > > Cc: Bin Meng > > Subject: [PATCH 15/18] riscv: qemu: Enable PRE_CONSOLE_BUFFER > > > > By default the video console only outputs messages after it's ready. > > Messages before that won't show on the video console, but U-Boot has an > > option to buffer the console messages before it's ready. > > > > Enable this support, and carefully select an address for the buffer. > > > > Signed-off-by: Bin Meng > > --- > > > > board/emulation/qemu-riscv/Kconfig | 5 + > > 1 file changed, 5 insertions(+) > > Reviewed-by: Rick Chen Reviewed-by: Simon Glass
Re: [PATCH 10/18] riscv: qemu: Enable Bochs video support
On Sat, 22 Jul 2023 at 22:41, Bin Meng wrote: > > Enable video console using the emulated Bochs VGA card. > > Signed-off-by: Bin Meng > --- > > board/emulation/qemu-riscv/Kconfig | 3 +++ > doc/board/emulation/qemu-riscv.rst | 5 + > include/configs/qemu-riscv.h | 5 + > 3 files changed, 13 insertions(+) Reviewed-by: Simon Glass How about a series to move this over to txt environment?
Re: [PATCH 17/18] riscv: qemu: Remove out-of-date "riscv, kernel-start" handling
On Tue, 25 Jul 2023 at 01:17, Rick Chen wrote:
>
> > From: U-Boot On Behalf Of Bin Meng
> > Sent: Sunday, July 23, 2023 12:41 PM
> > To: Simon Glass ; u-boot@lists.denx.de
> > Cc: Bin Meng
> > Subject: [PATCH 17/18] riscv: qemu: Remove out-of-date "riscv,
> > kernel-start" handling
> >
> > Commit 66ffe57 ("riscv: qemu: detect and boot the kernel passed by QEMU")
> > added some logic to handle "riscv,kernel-start" in DT and stored the
> > address to an environment variable kernel_start.
> >
> > However this "riscv,kernel-start" has never been an upstream DT binding.
> > The upstream QEMU never generates such a DT either. Presumably U-Boot
> > development was based on a downstream QEMU fork.
> >
> > Now we drop all codes in commit 66ffe57, except that BOARD_LATE_INIT is
> > kept for later use.
> >
> > Signed-off-by: Bin Meng
> > ---
> >
> > board/emulation/qemu-riscv/qemu-riscv.c | 24
> > include/configs/qemu-riscv.h| 10 --
> > 2 files changed, 34 deletions(-)
>
> Reviewed-by: Rick Chen
Reviewed-by: Simon Glass
Re: [PATCH] fdt: off by one in ofnode_lookup_fdt()
Hi Dan,
On Wed, 26 Jul 2023 at 00:59, Dan Carpenter wrote:
>
> The "oftree_count" is the number of entries which have been set in
> the oftree_list[] array. If all the entries have been initialized then
> this off by one would result in reading one element beyond the end
> of the array.
>
> Signed-off-by: Dan Carpenter
> ---
> drivers/core/ofnode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass
Thanks. It can be helpful to add 'Fixes:' tags on such patches.
>
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index 8df16e56af5c..a4dc9bde085c 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -103,7 +103,7 @@ void *ofnode_lookup_fdt(ofnode node)
> if (gd->flags & GD_FLG_RELOC) {
> uint i = OFTREE_TREE_ID(node.of_offset);
>
> - if (i > oftree_count) {
> + if (i >= oftree_count) {
> log_debug("Invalid tree ID %x\n", i);
> return NULL;
> }
> --
> 2.39.2
>
Regards,
Simon
Re: Securing u-boot: allow only authentic images
Hi, On Tue, 25 Jul 2023 at 09:40, Martin van den Berg wrote: > > Hi there, > > I'm new to u-boot and in need for a little assistance, I hope someone can > point me in the right direction. > > I need to secure the bootloader of a device to some extend. The device is > currently using u-boot as bootloader and I would like to stick with that. > > The device runs an OpenWRT. The SoC is a HLK7628N. > > At this moment, it is possible to use the u-boot bootloader to replace the > image of the device with any other image. I would like to have u-boot to > allow only authentic (signed?) images. What is the best way to accomplish > this? > Any pointers, examples and so on will be much appreciated. https://u-boot.readthedocs.io/en/latest/usage/fit/signature.html You can also find various talks on this topic, some linked from https://u-boot.readthedocs.io/en/latest/learn/index.html If you find any others that are interesting, please do add them to elinux.org Regards, Simon
RE: [PATCH v1] HSD #18028953892: usb: xhci-dwc3: Fix USB3.1 controller register access in reset state
Hi > -Original Message- > From: Marek Vasut > Sent: Wednesday, 26 July, 2023 6:30 PM > To: Chong, Teik Heng ; Lim, Jit Loon > ; u-boot@lists.denx.de > Cc: Jagan Teki ; Simon > ; Chee, Tien Fong > ; Hea, Kok Kiang ; > Maniyam, Dinesh ; Ng, Boon Khai > ; Yuslaimi, Alif Zakuan > ; Zamri, Muhammad Hazim Izzat > ; Tang, Sieu Mun > ; Bin Meng ; Michal > Simek ; Tom Rini ; Eugen Hristev > > Subject: Re: [PATCH v1] HSD #18028953892: usb: xhci-dwc3: Fix USB3.1 > controller register access in reset state > > On 7/26/23 06:04, Chong, Teik Heng wrote: > [...] > > Linux: (__dwc3_of_simple_teardown) > > https://elixir.bootlin.com/linux/latest/source/drivers/usb/dwc3/dw > > c3 > > -o > > f-simple.c#L98 > > U-Boot: (xhci_dwc3_remove) > > https://elixir.bootlin.com/u-boot/latest/source/drivers/usb/host/x > > hc > > i- > > dwc3.c#L227 > > > > So we believed that we can't directly pickup all Linux kernel dwc3 > > patches > and merge to U-Boot. > > If you were to sync the driver from Linux to U-Boot, then the same > sequence as Linux uses would be automatically used too, right ? > > Sorry for the abysmal delay in my reply. > >>> > >>> Are we saying that we shall port/use Linux driver in U-Boot and > >>> abandon > >> the existing USB host driver in U-Boot? > >> > >> No, the existing driver in U-Boot is a port of the Linux driver, it > >> is just outdated. I am saying, just pick the missing patches from > >> Linux and add them to U-Boot, so the two drivers are in sync. > > > > We do not see a DWC3 host driver under usb host folder in Linux > > https://elixir.bootlin.com/linux/latest/source/drivers/usb/host. Would > > you mind to tell us which DWC3 host driver we should use in usb host > > folder? Then, we can pick up the missing patches > > drivers/usb/dwc3 > > Same as u-boot Ok. We will use the driver from drivers/usb/dwc3. Thank you for the pointers
Re: Building and Running UBoot on UBuntu Linux distribution
Hi, On Wed, 26 Jul 2023 at 11:23, Ritika Chauhan wrote: > > Hello Team, > I am writing in since I am learning about bootloader, so I tried building > Das U-boot source code on WSL Ubuntu distribution but could not find any > guide or documentation. So, I followed the one with Debian linux > distribution, I installed gcc and gcc cross-compiler and tried installing > the packages as mentioned in the document but they didnt install. > > I want your help in guiding me on how to build and run the Das U-boot > source code on WSL linux distribution. > Looking forward to your replying asap. Have you tried here? https://u-boot.readthedocs.io/en/latest/build/index.html Regards, Simon
Re: [PATCH 1/5] adc: Use regulator_set_enable_if_allowed
On Wed, 19 Jul 2023 at 15:21, Jonas Karlman wrote:
>
> With the commit 4fcba5d556b4 ("regulator: implement basic reference
> counter") the return value of regulator_set_enable may be EALREADY or
> EBUSY for fixed/gpio regulators.
>
> Change to use the more relaxed regulator_set_enable_if_allowed to
> continue if regulator already was enabled or disabled.
>
> Signed-off-by: Jonas Karlman
> ---
> drivers/adc/adc-uclass.c | 22 ++
> 1 file changed, 10 insertions(+), 12 deletions(-)
Reviewed-by: Simon Glass
Tested-by: Simon Glass # rockpro64-rk3399
Re: [PATCH 1/1] virtio: provide driver name in debug message
On Wed, 26 Jul 2023 at 11:00, Heinrich Schuchardt wrote: > > If a driver cannot be bound, provide the driver name in the debug > message. Now the debug message may look like this: > > (virtio-pci.l#0): virtio-rng driver not configured > > Signed-off-by: Heinrich Schuchardt > --- > drivers/virtio/virtio-uclass.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH] addrmap: Fix off by one in addrmap_set_entry()
Hi Dan,
On Tue, 25 Jul 2023 at 09:40, Dan Carpenter wrote:
>
> The > comparison needs to be changed to >= to prevent an out of bounds
> write on th next line.
>
> Signed-off-by: Dan Carpenter
> ---
> lib/addr_map.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/addr_map.c b/lib/addr_map.c
> index 9b3e0a544e47..86e932e4b561 100644
> --- a/lib/addr_map.c
> +++ b/lib/addr_map.c
> @@ -59,7 +59,7 @@ void *addrmap_phys_to_virt(phys_addr_t paddr)
> void addrmap_set_entry(unsigned long vaddr, phys_addr_t paddr,
> phys_size_t size, int idx)
> {
> - if (idx > CONFIG_SYS_NUM_ADDR_MAP)
> + if (idx >= CONFIG_SYS_NUM_ADDR_MAP)
> return;
It looks like this function should return an error.
>
> address_map[idx].vaddr = vaddr;
> --
> 2.39.2
>
Regards,
Simon
Re: [PATCH] cros_ec: Fix an error code is cros_ec_get_sku_id()
On Wed, 26 Jul 2023 at 00:58, Dan Carpenter wrote: > > The ec_command_inptr() function returns negative error codes or > the number of bytes that it was able to read. The cros_ec_get_sku_id() > function should return negative error codes. Right now it returns > positive error codes or negative byte counts. > > Signed-off-by: Dan Carpenter > --- > drivers/misc/cros_ec.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH] regulator: preserve error code properly in regulator_list_autoset()
On Wed, 26 Jul 2023 at 01:01, Dan Carpenter wrote: > > This code has a & vs && typo so it only preserves odd value error > codes and not even value error codes. > > Signed-off-by: Dan Carpenter > --- > drivers/power/regulator/regulator-uclass.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH] efi_loader: Allow also empty capsule to be process
Hi Michal, Sughosh, On Wed, Jul 26, 2023 at 06:36:56PM +0200, Michal Simek wrote: > > > On 7/26/23 15:07, Ilias Apalodimas wrote: > > Hi all > > > > [...] > > > > > > > > > > > > > > > Hello Sugosh, > > > > > > > > fwu_empty_capsule() detects an empty capsule as one with a GUID > > > > fwu_guid_os_request_fw_revert or fwu_guid_os_request_fw_accept. > > > > > > > > I am not aware of a requirement in the UEFI specification to treat > > > > capsules read from file in a different way than capsules passed via > > > > UpdateCapsule(). Is there any reason why UpdateCapsule() should not > > > > process an empty capsule when called from a boot-time EFI application? > > > > > > Here is a story behind efi_update_capsule(): > > > === > > > commit a6aafce494ab > > > Author: Masami Hiramatsu > > > Date: Wed Feb 16 15:15:42 2022 +0900 > > > > > > efi_loader: use efi_update_capsule_firmware() for capsule on disk > > > === > > > > > > I still believe that this is a valid change, but we should have > > > moved 'capsule->capsule_guid' check into efi_update_capsule_firmware() > > > at the same time. > > > > I agree with Akashi-san here. I am also fine with this patchset since > > running the A/B update from an EFI app should work. But can we do a v2 > > with 2 patches? > > #1 move the capsule check along with the empty capsule checks in > > efi_update_capsule_firmware() > > #2 fix the a/b updates via the runtime calls and adjust the commit > > message accordingly, explaining why this change is needed? > > Can someone from Linaro create v2 on this? > I just wanted to pointed to it. Yes, I posted "efi_loader: capsule: enforce guid check in api and capsule_on_disk". Since I didn't run any test against A/B update, can you please confirm that this patch works in your environment? Unlike Ilias suggested, I made a single patch because an empty capsule will be handled any way at the beginning of efi_capsule_update_firmware() and we process only normal capsules after that. -Takahiro Akashi > Thanks, > Michal
[PATCH] efi_loader: capsule: enforce guid check in api and capsule_on_disk
While UPDATE_CAPSULE api is not fully implemented, this interface and
capsule-on-disk feature should behave in the same way, especially in
handling an empty capsule for fwu multibank, for future enhancement.
So move the guid check into efi_capsule_update_firmware().
Fixed: commit a6aafce494ab ("efi_loader: use efi_update_capsule_firmware()
for capsule on disk")
Reported-by: Michal Simek
Signed-off-by: AKASHI Takahiro
---
lib/efi_loader/efi_capsule.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 7a6f195cbc02..ddf8153e0982 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -581,6 +581,13 @@ static efi_status_t efi_capsule_update_firmware(
fw_accept_os = capsule_data->flags & FW_ACCEPT_OS ? 0x1 : 0x0;
}
+ if (guidcmp(&capsule_data->capsule_guid,
+ &efi_guid_firmware_management_capsule_id)) {
+ log_err("Unsupported capsule type: %pUs\n",
+ &capsule_data->capsule_guid);
+ return EFI_UNSUPPORTED;
+ }
+
/* sanity check */
if (capsule_data->header_size < sizeof(*capsule) ||
capsule_data->header_size >= capsule_data->capsule_image_size)
@@ -751,15 +758,7 @@ efi_status_t EFIAPI efi_update_capsule(
log_debug("Capsule[%d] (guid:%pUs)\n",
i, &capsule->capsule_guid);
- if (!guidcmp(&capsule->capsule_guid,
-&efi_guid_firmware_management_capsule_id)) {
- ret = efi_capsule_update_firmware(capsule);
- } else {
- log_err("Unsupported capsule type: %pUs\n",
- &capsule->capsule_guid);
- ret = EFI_UNSUPPORTED;
- }
-
+ ret = efi_capsule_update_firmware(capsule);
if (ret != EFI_SUCCESS)
goto out;
}
--
2.41.0
Re: [RFC] efi_driver: fix a parent issue in efi-created block devices
Please ignore the following RFC since I accidentally posted the wrong patch.
-Takahiro Akashi
On Thu, Jul 27, 2023 at 09:29:55AM +0900, AKASHI Takahiro wrote:
> An EFI application may create an EFI block device (EFI_BLOCK_IO_PROTOCOL) in
> EFI world, which in turn generates a corresponding U-Boot block device based
> on
> U-Boot's Driver Model.
> The latter device, however, doesn't work as U-Boot proper block device
> due to an issue in efi_driver's implementation. We saw discussions in the
> past,
> most recently in [1].
>
> [1] https://lists.denx.de/pipermail/u-boot/2023-July/522565.html
>
> This RFC patch tries to address (part of) the issue.
> If it is considered acceptable, I will create a formal patch.
>
> Withtout this patch,
> ===8<===
> => env set efi_selftest 'block device'
> => bootefi selftest
> ...
>
> Summary: 0 failures
>
> => dm tree
> Class Index Probed DriverName
> ---
> root 0 [ + ] root_driver root_driver
> ...
> bootmeth 7 [ ] vbe_simple| `-- vbe_simple
> blk 0 [ + ] efi_blk `-- efiblk#0
> partition 0 [ + ] blk_partition `-- efiblk#0:1
> => ls efiloader 0:1
> ** Bad device specification efiloader 0 **
> Couldn't find partition efiloader 0:1
> ===>8===
>
> With this patch applied, efiblk#0(:1) now gets accessible.
>
> ===8<===
> => env set efi_selftest 'block device'
> => bootefi selftest
> ...
>
> Summary: 0 failures
>
> => dm tree
> Class Index Probed DriverName
> ---
> root 0 [ + ] root_driver root_driver
> ...
> bootmeth 7 [ ] vbe_simple| `-- vbe_simple
> efi 0 [ + ] EFI block driver `--
> /VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)
> blk 0 [ + ] efi_blk `-- efiblk#0
> partition 0 [ + ] blk_partition `-- efiblk#0:1
> => ls efiloader 0:1
>13 hello.txt
> 7 u-boot.txt
>
> 2 file(s), 0 dir(s)
> ===>8===
>
> Signed-off-by: AKASHI Takahiro
> ---
> include/efi_driver.h | 2 +-
> lib/efi_driver/efi_block_device.c| 17 -
> lib/efi_driver/efi_uclass.c | 8 +++-
> lib/efi_selftest/efi_selftest_block_device.c | 2 ++
> 4 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/include/efi_driver.h b/include/efi_driver.h
> index 63a95e4cf800..ed66796f3519 100644
> --- a/include/efi_driver.h
> +++ b/include/efi_driver.h
> @@ -42,7 +42,7 @@ struct efi_driver_ops {
> const efi_guid_t *child_protocol;
> efi_status_t (*init)(struct efi_driver_binding_extended_protocol *this);
> efi_status_t (*bind)(struct efi_driver_binding_extended_protocol *this,
> - efi_handle_t handle, void *interface);
> + efi_handle_t handle, void *interface, char *name);
> };
>
> #endif /* _EFI_DRIVER_H */
> diff --git a/lib/efi_driver/efi_block_device.c
> b/lib/efi_driver/efi_block_device.c
> index add00eeebbea..43b7ed7c973c 100644
> --- a/lib/efi_driver/efi_block_device.c
> +++ b/lib/efi_driver/efi_block_device.c
> @@ -115,9 +115,9 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t
> blknr, lbaint_t blkcnt,
> * Return: status code
> */
> static efi_status_t
> -efi_bl_create_block_device(efi_handle_t handle, void *interface)
> +efi_bl_create_block_device(efi_handle_t handle, void *interface, struct
> udevice *parent)
> {
> - struct udevice *bdev = NULL, *parent = dm_root();
> + struct udevice *bdev = NULL;
> efi_status_t ret;
> int devnum;
> char *name;
> @@ -181,7 +181,7 @@ err:
> */
> static efi_status_t efi_bl_bind(
> struct efi_driver_binding_extended_protocol *this,
> - efi_handle_t handle, void *interface)
> + efi_handle_t handle, void *interface, char *name)
> {
> efi_status_t ret = EFI_SUCCESS;
> struct efi_object *obj = efi_search_obj(handle);
> @@ -191,8 +191,15 @@ static efi_status_t efi_bl_bind(
> if (!obj || !interface)
> return EFI_INVALID_PARAMETER;
>
> - if (!handle->dev)
> - ret = efi_bl_create_block_device(handle, interface);
> + if (!handle->dev) {
> + struct udevice *parent;
> +
> + ret = device_bind_driver(dm_root(), "EFI block driver", name,
> &parent);
> + if (!ret)
> + ret = efi_bl_create_block_device(handle, interface,
> parent);
> + else
> + ret = EFI_DEVICE_ERROR;
> + }
>
> return ret;
> }
> diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
> index 45f935198874..bf669742783e 100644
> --- a/lib/efi_driver/efi_uclass.c
> +++ b/lib/efi_driver/efi_ucla
[RFC] efi_driver: fix a parent issue in efi-created block devices
An EFI application may create an EFI block device (EFI_BLOCK_IO_PROTOCOL) in
EFI world, which in turn generates a corresponding U-Boot block device based on
U-Boot's Driver Model.
The latter device, however, doesn't work as U-Boot proper block device
due to an issue in efi_driver's implementation. We saw discussions in the past,
most recently in [1].
[1] https://lists.denx.de/pipermail/u-boot/2023-July/522565.html
This RFC patch tries to address (part of) the issue.
If it is considered acceptable, I will create a formal patch.
Withtout this patch,
===8<===
=> env set efi_selftest 'block device'
=> bootefi selftest
...
Summary: 0 failures
=> dm tree
Class Index Probed DriverName
---
root 0 [ + ] root_driver root_driver
...
bootmeth 7 [ ] vbe_simple| `-- vbe_simple
blk 0 [ + ] efi_blk `-- efiblk#0
partition 0 [ + ] blk_partition `-- efiblk#0:1
=> ls efiloader 0:1
** Bad device specification efiloader 0 **
Couldn't find partition efiloader 0:1
===>8===
With this patch applied, efiblk#0(:1) now gets accessible.
===8<===
=> env set efi_selftest 'block device'
=> bootefi selftest
...
Summary: 0 failures
=> dm tree
Class Index Probed DriverName
---
root 0 [ + ] root_driver root_driver
...
bootmeth 7 [ ] vbe_simple| `-- vbe_simple
efi 0 [ + ] EFI block driver `--
/VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)
blk 0 [ + ] efi_blk `-- efiblk#0
partition 0 [ + ] blk_partition `-- efiblk#0:1
=> ls efiloader 0:1
13 hello.txt
7 u-boot.txt
2 file(s), 0 dir(s)
===>8===
Signed-off-by: AKASHI Takahiro
---
include/efi_driver.h | 2 +-
lib/efi_driver/efi_block_device.c| 17 -
lib/efi_driver/efi_uclass.c | 8 +++-
lib/efi_selftest/efi_selftest_block_device.c | 2 ++
4 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/include/efi_driver.h b/include/efi_driver.h
index 63a95e4cf800..ed66796f3519 100644
--- a/include/efi_driver.h
+++ b/include/efi_driver.h
@@ -42,7 +42,7 @@ struct efi_driver_ops {
const efi_guid_t *child_protocol;
efi_status_t (*init)(struct efi_driver_binding_extended_protocol *this);
efi_status_t (*bind)(struct efi_driver_binding_extended_protocol *this,
-efi_handle_t handle, void *interface);
+efi_handle_t handle, void *interface, char *name);
};
#endif /* _EFI_DRIVER_H */
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index add00eeebbea..43b7ed7c973c 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -115,9 +115,9 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t
blknr, lbaint_t blkcnt,
* Return: status code
*/
static efi_status_t
-efi_bl_create_block_device(efi_handle_t handle, void *interface)
+efi_bl_create_block_device(efi_handle_t handle, void *interface, struct
udevice *parent)
{
- struct udevice *bdev = NULL, *parent = dm_root();
+ struct udevice *bdev = NULL;
efi_status_t ret;
int devnum;
char *name;
@@ -181,7 +181,7 @@ err:
*/
static efi_status_t efi_bl_bind(
struct efi_driver_binding_extended_protocol *this,
- efi_handle_t handle, void *interface)
+ efi_handle_t handle, void *interface, char *name)
{
efi_status_t ret = EFI_SUCCESS;
struct efi_object *obj = efi_search_obj(handle);
@@ -191,8 +191,15 @@ static efi_status_t efi_bl_bind(
if (!obj || !interface)
return EFI_INVALID_PARAMETER;
- if (!handle->dev)
- ret = efi_bl_create_block_device(handle, interface);
+ if (!handle->dev) {
+ struct udevice *parent;
+
+ ret = device_bind_driver(dm_root(), "EFI block driver", name,
&parent);
+ if (!ret)
+ ret = efi_bl_create_block_device(handle, interface,
parent);
+ else
+ ret = EFI_DEVICE_ERROR;
+ }
return ret;
}
diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
index 45f935198874..bf669742783e 100644
--- a/lib/efi_driver/efi_uclass.c
+++ b/lib/efi_driver/efi_uclass.c
@@ -145,7 +145,13 @@ static efi_status_t EFIAPI efi_uc_start(
ret = check_node_type(controller_handle);
if (ret != EFI_SUCCESS)
goto err;
- ret = bp->ops->bind(bp, controller_handle, interface);
+
+ struct efi_handler *handler;
+ char tmpname[256] = "AppName";
+ ret = efi_search_protocol(controller_handle, &efi
Re: [PATCH] sunxi: remove CONFIG_SATAPWR
On 7/26/23 17:47, Andre Przywara wrote:
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b3115b054c8..422cc01e529 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1158,6 +1158,8 @@ config ARCH_SUNXI
imply CMD_GPT
imply CMD_UBI if MTD_RAW_NAND
imply DISTRO_DEFAULTS
+ imply DM_REGULATOR
+ imply DM_REGULATOR_FIXED
imply FAT_WRITE
imply FIT
imply OF_LIBFDT_OVERLAY
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index e20c3a3ee92..a7c5ae80a1e 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -1008,14 +1008,6 @@ config VIDEO_LCD_TL059WV5C0
endchoice
-config SATAPWR
- string "SATA power pin"
- default ""
- help
- Set the pins used to power the SATA. This takes a string in the
- format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of
- port H.
-
config GMAC_TX_DELAY
int "GMAC Transmit Clock Delay Chain"
default 0
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index f321cd58a6e..7ac50c4ae8c 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -187,7 +187,7 @@ enum env_location env_get_location(enum env_operation op,
int prio)
/* add board specific code here */
int board_init(void)
{
- __maybe_unused int id_pfr1, ret, satapwr_pin, macpwr_pin;
+ __maybe_unused int id_pfr1, ret, macpwr_pin;
gd->bd->bi_boot_params = (PHYS_SDRAM_0 + 0x100);
@@ -225,20 +225,6 @@ int board_init(void)
return ret;
/* strcmp() would look better, but doesn't get optimised away. */
- if (CONFIG_SATAPWR[0]) {
- satapwr_pin = sunxi_name_to_gpio(CONFIG_SATAPWR);
- if (satapwr_pin >= 0) {
- gpio_request(satapwr_pin, "satapwr");
- gpio_direction_output(satapwr_pin, 1);
-
- /*
-* Give the attached SATA device time to power-up
-* to avoid link timeouts
-*/
- mdelay(500);
- }
- }
-
if (CONFIG_MACPWR[0]) {
macpwr_pin = sunxi_name_to_gpio(CONFIG_MACPWR);
if (macpwr_pin >= 0) {
...
diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
index 94a3379c532..9064774e661 100644
--- a/drivers/ata/ahci_sunxi.c
+++ b/drivers/ata/ahci_sunxi.c
@@ -7,6 +7,7 @@
#include
#include
#include
+#include
#define AHCI_PHYCS0R 0x00c0
#define AHCI_PHYCS1R 0x00c4
@@ -74,6 +75,7 @@ static int sunxi_ahci_phy_init(u8 *reg_base)
static int sunxi_sata_probe(struct udevice *dev)
{
+ struct udevice *reg_dev;
ulong base;
u8 *reg;
int ret;
@@ -89,6 +91,13 @@ static int sunxi_sata_probe(struct udevice *dev)
debug("%s: Failed to init phy (err=%d)\n", __func__, ret);
return ret;
}
+
+ ret = device_get_supply_regulator(dev, "target-supply", ®_dev);
+ if (ret == 0) {
+ regulator_set_enable(reg_dev, true);
+ mdelay(500);
+ }
+
ret = ahci_probe_scsi(dev, base);
if (ret) {
debug("%s: Failed to probe (err=%d)\n", __func__, ret);
Love these sorts of clean-ups. I don't have a sunxi with SATA so I can't
test it, but I've been running my target on this patch in some form or
another for several weeks, and the code looks good, so:
Reviewed-by: Sam Edwards
[PATCH v2] ARM: dts: stm32: Switch DWMAC RMII clock to MCO2 on DHCOM
The DHCOM SoM has two options for supplying ETHRX clock to the DWMAC
block and PHY. Either (1) ETHCK_K generates 50 MHz clock on ETH_CLK
pad for the PHY and the same 50 MHz clock are fed back to ETHRX via
internal eth_clk_fb clock connection OR (2) ETH_CLK is not used at
all, MCO2 generates 50 MHz clock on MCO2 output pad for the PHY and
the same MCO2 clock are fed back into ETHRX via ETH_RX_CLK input pad
using external pad-to-pad connection.
Option (1) has two downsides. ETHCK_K is supplied directly from either
PLL3_Q or PLL4_P, hence the PLL output is limited to exactly 50 MHz and
since the same PLL output is also used to supply SDMMC blocks, the
performance of SD and eMMC access is affected. The second downside is
that using this option, the EMI of the SoM is higher.
Option (2) solves both of those problems, so implement it here. In this
case, the PLL4_P is no longer limited and can be operated faster, at
100 MHz, which improves SDMMC performance (read performance is improved
from ~41 MiB/s to ~57 MiB/s with dd if=/dev/mmcblk1 of=/dev/null bs=64M
count=1). The EMI interference also decreases.
Ported from Linux kernel commit
73ab99aad50cd ("ARM: dts: stm32: Switch DWMAC RMII clock to MCO2 on DHCOM")
Signed-off-by: Marek Vasut
---
Cc: Patrice Chotard
Cc: Patrick Delaunay
Cc: u-b...@dh-electronics.com
Cc: uboot-st...@st-md-mailman.stormreply.com
---
V2: Add U-Boot specifics to cater also for clock and clock-names in
stm32mp151.dtsi
---
arch/arm/dts/stm32mp15xx-dhcom-som.dtsi| 22 ++
arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi | 14 ++
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/stm32mp15xx-dhcom-som.dtsi
b/arch/arm/dts/stm32mp15xx-dhcom-som.dtsi
index 83e2c87713f..238a611192e 100644
--- a/arch/arm/dts/stm32mp15xx-dhcom-som.dtsi
+++ b/arch/arm/dts/stm32mp15xx-dhcom-som.dtsi
@@ -118,13 +118,12 @@
ðernet0 {
status = "okay";
- pinctrl-0 = <ðernet0_rmii_pins_a>;
- pinctrl-1 = <ðernet0_rmii_sleep_pins_a>;
+ pinctrl-0 = <ðernet0_rmii_pins_c &mco2_pins_a>;
+ pinctrl-1 = <ðernet0_rmii_sleep_pins_c &mco2_sleep_pins_a>;
pinctrl-names = "default", "sleep";
phy-mode = "rmii";
max-speed = <100>;
phy-handle = <&phy0>;
- st,eth-ref-clk-sel;
mdio0 {
#address-cells = <1>;
@@ -136,7 +135,7 @@
/* LAN8710Ai */
compatible = "ethernet-phy-id0007.c0f0",
"ethernet-phy-ieee802.3-c22";
- clocks = <&rcc ETHCK_K>;
+ clocks = <&rcc CK_MCO2>;
reset-gpios = <&gpioh 3 GPIO_ACTIVE_LOW>;
reset-assert-us = <500>;
reset-deassert-us = <500>;
@@ -446,6 +445,21 @@
};
};
+&rcc {
+ /* Connect MCO2 output to ETH_RX_CLK input via pad-pad connection */
+ clocks = <&rcc CK_MCO2>;
+ clock-names = "ETH_RX_CLK/ETH_REF_CLK";
+
+ /*
+* Set PLL4P output to 100 MHz to supply SDMMC with faster clock,
+* set MCO2 output to 50 MHz to supply ETHRX clock with PLL4P/2,
+* so that MCO2 behaves as a divider for the ETHRX clock here.
+*/
+ assigned-clocks = <&rcc CK_MCO2>, <&rcc PLL4_P>;
+ assigned-clock-parents = <&rcc PLL4_P>;
+ assigned-clock-rates = <5000>, <1>;
+};
+
&rng1 {
status = "okay";
};
diff --git a/arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi
b/arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi
index bc0730cf2bd..ff5e9034951 100644
--- a/arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi
@@ -126,6 +126,20 @@
};
&rcc {
+ /*
+* Reinstate clock names from stm32mp151.dtsi, the MCO2 trick
+* used in stm32mp15xx-dhcom-som.dtsi is not supported by the
+* U-Boot clock framework.
+*/
+ clock-names = "hse", "hsi", "csi", "lse", "lsi";
+ clocks = <&clk_hse>, <&clk_hsi>, <&clk_csi>,
+<&clk_lse>, <&clk_lsi>;
+
+ /* The MCO2 is already configured correctly, remove those. */
+ /delete-property/ assigned-clocks;
+ /delete-property/ assigned-clock-parents;
+ /delete-property/ assigned-clock-rates;
+
st,clksrc = <
CLK_MPU_PLL1P
CLK_AXI_PLL2P
--
2.40.1
[PATCH] sunxi: remove CONFIG_SATAPWR
The CONFIG_SATAPWR Kconfig symbol was used to point to a GPIO that
enables the power for a SATA harddisk.
In the DT this is described with the target-supply property in the AHCI
DT node, pointing to a (GPIO controlled) regulator. Since we need SATA
only in U-Boot proper, and use a DM driver for AHCI there, we should use
the DT instead of hardcoding this.
Add code to the sunxi AHCI driver to check the DT for that regulator and
enable it, at probe time. Then drop the current code from board.c, which
was doing that job before.
This allows us to remove the SATAPWR Kconfig definition and the
respective values from the defconfigs.
We also select the generic fixed regulator driver, which handles those
GPIO controlled regulators.
Signed-off-by: Andre Przywara
---
Hi, this patch is actually the first patch in the T113s support series,
but was missing from the post. Without it the next patches won't apply
cleanly. It's part of the gitlab branch:
https://source.denx.de/u-boot/custodians/u-boot-sunxi/-/commits/t113s/
Cheers,
Andre
arch/arm/Kconfig | 2 ++
arch/arm/mach-sunxi/Kconfig | 8
board/sunxi/board.c | 16 +---
configs/A10-OLinuXino-Lime_defconfig | 1 -
configs/A20-OLinuXino-Lime2-eMMC_defconfig | 1 -
configs/A20-OLinuXino-Lime2_defconfig| 1 -
configs/A20-OLinuXino-Lime_defconfig | 1 -
configs/A20-OLinuXino_MICRO-eMMC_defconfig | 1 -
configs/A20-OLinuXino_MICRO_defconfig| 1 -
configs/A20-Olimex-SOM-EVB_defconfig | 1 -
configs/A20-Olimex-SOM204-EVB-eMMC_defconfig | 1 -
configs/A20-Olimex-SOM204-EVB_defconfig | 1 -
configs/Cubieboard2_defconfig| 1 -
configs/Cubieboard_defconfig | 1 -
configs/Cubietruck_defconfig | 1 -
configs/Itead_Ibox_A20_defconfig | 1 -
configs/Lamobo_R1_defconfig | 1 -
configs/Linksprite_pcDuino3_Nano_defconfig | 1 -
configs/Linksprite_pcDuino3_defconfig| 1 -
configs/Sinovoip_BPI_M3_defconfig| 1 -
configs/orangepi_plus_defconfig | 2 +-
drivers/ata/ahci_sunxi.c | 9 +
22 files changed, 13 insertions(+), 41 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b3115b054c8..422cc01e529 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1158,6 +1158,8 @@ config ARCH_SUNXI
imply CMD_GPT
imply CMD_UBI if MTD_RAW_NAND
imply DISTRO_DEFAULTS
+ imply DM_REGULATOR
+ imply DM_REGULATOR_FIXED
imply FAT_WRITE
imply FIT
imply OF_LIBFDT_OVERLAY
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index e20c3a3ee92..a7c5ae80a1e 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -1008,14 +1008,6 @@ config VIDEO_LCD_TL059WV5C0
endchoice
-config SATAPWR
- string "SATA power pin"
- default ""
- help
- Set the pins used to power the SATA. This takes a string in the
- format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of
- port H.
-
config GMAC_TX_DELAY
int "GMAC Transmit Clock Delay Chain"
default 0
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index f321cd58a6e..7ac50c4ae8c 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -187,7 +187,7 @@ enum env_location env_get_location(enum env_operation op,
int prio)
/* add board specific code here */
int board_init(void)
{
- __maybe_unused int id_pfr1, ret, satapwr_pin, macpwr_pin;
+ __maybe_unused int id_pfr1, ret, macpwr_pin;
gd->bd->bi_boot_params = (PHYS_SDRAM_0 + 0x100);
@@ -225,20 +225,6 @@ int board_init(void)
return ret;
/* strcmp() would look better, but doesn't get optimised away. */
- if (CONFIG_SATAPWR[0]) {
- satapwr_pin = sunxi_name_to_gpio(CONFIG_SATAPWR);
- if (satapwr_pin >= 0) {
- gpio_request(satapwr_pin, "satapwr");
- gpio_direction_output(satapwr_pin, 1);
-
- /*
-* Give the attached SATA device time to power-up
-* to avoid link timeouts
-*/
- mdelay(500);
- }
- }
-
if (CONFIG_MACPWR[0]) {
macpwr_pin = sunxi_name_to_gpio(CONFIG_MACPWR);
if (macpwr_pin >= 0) {
diff --git a/configs/A10-OLinuXino-Lime_defconfig
b/configs/A10-OLinuXino-Lime_defconfig
index df4fdfaba41..57e91d0f017 100644
--- a/configs/A10-OLinuXino-Lime_defconfig
+++ b/configs/A10-OLinuXino-Lime_defconfig
@@ -7,7 +7,6 @@ CONFIG_DRAM_CLK=480
CONFIG_DRAM_EMR1=4
CONFIG_SYS_CLK_FREQ=91200
CONFIG_I2C1_ENABLE=y
-CONFIG_SATAPWR="PC3"
CONFIG_AHCI=y
# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
CONFIG_SPL_I2C=y
diff --git a/configs/A20-OLinuXino-Lime2-e
[PATCH] ARM: dts: stm32: Switch DWMAC RMII clock to MCO2 on DHCOM
The DHCOM SoM has two options for supplying ETHRX clock to the DWMAC
block and PHY. Either (1) ETHCK_K generates 50 MHz clock on ETH_CLK
pad for the PHY and the same 50 MHz clock are fed back to ETHRX via
internal eth_clk_fb clock connection OR (2) ETH_CLK is not used at
all, MCO2 generates 50 MHz clock on MCO2 output pad for the PHY and
the same MCO2 clock are fed back into ETHRX via ETH_RX_CLK input pad
using external pad-to-pad connection.
Option (1) has two downsides. ETHCK_K is supplied directly from either
PLL3_Q or PLL4_P, hence the PLL output is limited to exactly 50 MHz and
since the same PLL output is also used to supply SDMMC blocks, the
performance of SD and eMMC access is affected. The second downside is
that using this option, the EMI of the SoM is higher.
Option (2) solves both of those problems, so implement it here. In this
case, the PLL4_P is no longer limited and can be operated faster, at
100 MHz, which improves SDMMC performance (read performance is improved
from ~41 MiB/s to ~57 MiB/s with dd if=/dev/mmcblk1 of=/dev/null bs=64M
count=1). The EMI interference also decreases.
Ported from Linux kernel commit
73ab99aad50cd ("ARM: dts: stm32: Switch DWMAC RMII clock to MCO2 on DHCOM")
Signed-off-by: Marek Vasut
---
Cc: Patrice Chotard
Cc: Patrick Delaunay
Cc: u-b...@dh-electronics.com
Cc: uboot-st...@st-md-mailman.stormreply.com
---
arch/arm/dts/stm32mp15xx-dhcom-som.dtsi | 22 ++
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/stm32mp15xx-dhcom-som.dtsi
b/arch/arm/dts/stm32mp15xx-dhcom-som.dtsi
index 83e2c87713f..238a611192e 100644
--- a/arch/arm/dts/stm32mp15xx-dhcom-som.dtsi
+++ b/arch/arm/dts/stm32mp15xx-dhcom-som.dtsi
@@ -118,13 +118,12 @@
ðernet0 {
status = "okay";
- pinctrl-0 = <ðernet0_rmii_pins_a>;
- pinctrl-1 = <ðernet0_rmii_sleep_pins_a>;
+ pinctrl-0 = <ðernet0_rmii_pins_c &mco2_pins_a>;
+ pinctrl-1 = <ðernet0_rmii_sleep_pins_c &mco2_sleep_pins_a>;
pinctrl-names = "default", "sleep";
phy-mode = "rmii";
max-speed = <100>;
phy-handle = <&phy0>;
- st,eth-ref-clk-sel;
mdio0 {
#address-cells = <1>;
@@ -136,7 +135,7 @@
/* LAN8710Ai */
compatible = "ethernet-phy-id0007.c0f0",
"ethernet-phy-ieee802.3-c22";
- clocks = <&rcc ETHCK_K>;
+ clocks = <&rcc CK_MCO2>;
reset-gpios = <&gpioh 3 GPIO_ACTIVE_LOW>;
reset-assert-us = <500>;
reset-deassert-us = <500>;
@@ -446,6 +445,21 @@
};
};
+&rcc {
+ /* Connect MCO2 output to ETH_RX_CLK input via pad-pad connection */
+ clocks = <&rcc CK_MCO2>;
+ clock-names = "ETH_RX_CLK/ETH_REF_CLK";
+
+ /*
+* Set PLL4P output to 100 MHz to supply SDMMC with faster clock,
+* set MCO2 output to 50 MHz to supply ETHRX clock with PLL4P/2,
+* so that MCO2 behaves as a divider for the ETHRX clock here.
+*/
+ assigned-clocks = <&rcc CK_MCO2>, <&rcc PLL4_P>;
+ assigned-clock-parents = <&rcc PLL4_P>;
+ assigned-clock-rates = <5000>, <1>;
+};
+
&rng1 {
status = "okay";
};
--
2.40.1
[PATCH] mtd: spi-nor: Add support for Silicon Kaiser sk25lp128
Add support for Silicon Kaiser sk25lp128 SPI NOR flash found in Pine64
PinePhone Pro and PineTab2.
Signed-off-by: Jonas Karlman
---
drivers/mtd/spi/Kconfig | 5 +
drivers/mtd/spi/spi-nor-ids.c | 4
2 files changed, 9 insertions(+)
diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
index a9617c6c58c1..4329b88520df 100644
--- a/drivers/mtd/spi/Kconfig
+++ b/drivers/mtd/spi/Kconfig
@@ -169,6 +169,11 @@ config SPI_FLASH_MACRONIX
help
Add support for various Macronix SPI flash chips (MX25Lxxx)
+config SPI_FLASH_SILICONKAISER
+ bool "Silicon Kaiser SPI flash support"
+ help
+ Add support for various Silicon Kaiser SPI flash chips (SK25Lxxx)
+
config SPI_FLASH_SPANSION
bool "Spansion SPI flash support"
help
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index 458721598424..c461c1b928d2 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -286,6 +286,10 @@ const struct flash_info spi_nor_ids[] = {
{ INFO("mx25uw6345g",0xc28437, 0, 64 * 1024, 128, SECT_4K |
SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) },
#endif
+#ifdef CONFIG_SPI_FLASH_SILICONKAISER
+ { INFO("sk25lp128", 0x257018, 0, 64 * 1024, 256, SECT_4K |
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+#endif
+
#ifdef CONFIG_SPI_FLASH_STMICRO/* STMICRO */
/* Micron */
{ INFO("n25q016a", 0x20bb15, 0, 64 * 1024, 32, SECT_4K |
SPI_NOR_QUAD_READ) },
--
2.41.0
Re: [PATCH] riscv: Support riscv64 image type
Hi Rick, On Wed, 19 Apr 2023 at 00:56, Rick Chen wrote: > > Hi Simon, > > > Hi Rick, > > > > On Mon, 10 Apr 2023 at 01:26, Rick Chen wrote: > > > > > > Allow U-Boot to load 32 or 64 bits RISC-V Kernel Image > > > distinguishly. It helps to avoid someone maybe make a mistake > > > to run 32-bit U-Boot to load 64-bit kernel. > > > > > > Signed-off-by: Rick Chen > > > > > > --- > > > The patchset is based on Simon's patch: > > > riscv: Add a 64-bit image type > > > --- > > > --- > > > arch/riscv/include/asm/u-boot.h | 4 > > > cmd/booti.c | 2 +- > > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > Reviewed-by: Simon Glass > > > > I don't know much about RISC-V, but I assume U-Boot is able to do this > > successfully? Does it not need to switch modes first? > > No, it is not need to switch modes as far as I know. > Here only provide a check mechanism just like arm to see if loader and > OS are match > > But This patch is for bootm flow. > Maybe I still need to check if it is necessary to prepare a patch for > binman flow ? > /arch/riscv/dts/binman.dtsi > arch = "riscv"; > > maybe provide another binman64.dtsi for arch="riscv64" Yes I think that is needed too. Are you going to update this patch, or send a second one? Regards, Simon
Re: [PATCH 0/4] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl
Maxime, On 7/20/23 7:42 PM, Maxime Ripard wrote: > Hi > > Sorry, I didn't receive Roger mail so I'll reply here > > On Thu, Jul 20, 2023 at 09:00:19AM -0500, Nishanth Menon wrote: >> On 16:56-20230720, Roger Quadros wrote: >>> Hi, >>> >>> On 20/07/2023 16:33, Ravi Gunasekaran wrote: On 7/20/2023 3:25 PM, Maxime Ripard wrote: > Hi, > > This series is based on: > https://lore.kernel.org/all/20230713072019.3153871-1...@ti.com/ > > It fixes the issue of Linux booting from the DT embedded by U-boot. The > main issue there is that U-Boot doesn't handle the MDIO child node that > might have resources attached to it. > > Thus, any pinctrl configuration that could be attached to the MDIO > child node is effectively ignored. Unfortunately, starting with 6.5-rc1, > Linux does just that. >>> >>> I didn't get this part. Linux does not ignore pinctrl configuration attached >>> to MDIO child node. What changed in 6.5-rc1? > > Linux doesn't ignore it, but Linux started putting pinctrl configuration > on the MDIO node, which is ignored by U-Boot. > > This was solved by duplicating the pinctrl configuration onto the MAC >>> >>> If I remember right, there is no driver model driver for MDIO in u-boot and >>> adding the mdio pinctrl into CPSW DT node was a hack in u-boot. > > Yeah, but we now have in the U-Boot DT two nodes referencing the same > pinctrl configuration: the CPSW and the MDIO node. Linux then tries to > assign that configuration to both devices and it fails. This response might be late, as I know things have moved ahead after this discussion. But I just wanted to understand the root cause little bit more. Is the issue mainly because the same mdio pinctrl node(phandle) is referenced in two other nodes? Or is it because of same pins being updated in two different places in the kernel? If it is the former, then would a duplicate mdio node help keep the changes to minimum? > > device node. Unfortunately, this doesn't work for Linux since now it has > two devices competing for the same pins. >>> >>> What has really changed here is that you are passing u-boot's device >>> tree to Linux. > > I didn't change anything. This is the default boot process if you don't > provide a DT in the ESP partition. > >>> This has nothing to do with 6.5-rc1 right? > > The issue started to occur with Nishanth patch to sync with Linux > 6.5-rc1 device trees, so there's definitely a connection. > >>> I suppose your solution is still a hack but of lesser evil than >>> duplicating MDIO pinctrl in CPSW node. >>> >>> The proper solution would be to implement driver model for the >>> davinci MDIO driver. Siddharth has been working on this. If that is >>> close to ready we should just use that instead. >> >> But this allows for a cleaner device tree while the driver can be fixed >> up independently, correct? Unfortunately, Siddharth's driver model work, >> from what I understand, is around 2024 timeframe.. which is probably not >> something that helps us in the community at this point. > > Yeah, at the moment I have to choose between getting the MMC to work in > Linux or the Ethernet ports. The former is working thanks to your patch, > the latter is broken because of it. Ideally I'd like both :) > > Maxime -- Regards, Ravi
Re: [PATCH 14/14] bloblist: Update documentation and header comment
Here are a couple of other differences I have found between the bloblist code after applying your patches and the TL specification: * bloblist seems to explicitly disallow having the same tag more than once in the list (e.g. see documentation of bloblist_add()), whereas the TL specification explicitly allows that. You're of course free to impose this restriction on the way U-Boot uses the spec, but you may eventually run into compatibility issues if you hardcode that assumption and one day might ingest a TL from a different writer that doesn't adhere to it. * The bloblist_resize() function doesn't preserve alignment restrictions when relocating other TEs. I think the (only?) safe way to resize a TE in-place would be to align the distance that following TEs need to be moved up to (1 << hdr->align_log2), and then insert a void dummy TE to account for the additional added distance if necessary. * The comment at the top of bloblist.c should be updated to reflect how alignment and padding works in the TL spec. * The checksum algorithm seems incorrect. U-Boot's table_compute_checksum() subtracts bytes, but the TE spec says they need to be summed up. * The bloblist_reloc() function must be updated to follow the TL relocation mechanism (i.e. take hdr->align_log2 into account to make sure the new position preserves alignment requirements... I would say see https://github.com/FirmwareHandoff/firmware_handoff/blob/main/source/transfer_list.rst#relocating-a-tl for details, but I just noticed that we made a mistake there, so please check the version from https://github.com/FirmwareHandoff/firmware_handoff/pull/12 for the corrected algorithm).
Re: [PATCH 3/6] board: ti: am62x: Add basic initialization for usb voltage, 32k crystal, debounce
On 00:35-20230726, Francesco Dolcini wrote: [...] > > > > > > At least the ones we have currently (I am not sure about toradex, > > > phytech etc), seem to operate the vdd_core at 0.85V .. (which is what > > > USB is dependent upon). > > > > For Toradex, we do have the equivalent code in our board file, see > > https://git.toradex.com/cgit/u-boot-toradex.git/tree/board/toradex/verdin-am62/verdin-am62.c?h=toradex_ti-u-boot-2023.04#n92 > > > > The 32kHz configuration is just different for us, we do not re-use the > > same you have here. True, you are hitting the bypass control and not powering on the oscillator control since the 32k is incoming from RTC, in my case, since I have an actual 32k crystal, I am clearing the powerdown. > > > > The debounce conf registers I have no idea what they are about, > > something we should have also on our board? Any additional details? > > So, I got curious and checked on the datasheet/TRM on this debounce. If > I understood correctly this is to have debounce on GPIO and/or EQEP. Typically, yes - input signals more useful for eQEP or GPIO. but the implementation is at pin level which, technically could be used for other purposes (but I have'nt seen any). > > However to my understanding this would need to have the corresponding > DEBOUNCE_SEL register written on the pad configuration, and by default it's 0. > > What's the use case for this debounce configuration you have here? TRM was a bit of a crap (internal ticket was filed to improve), but long story short: * bootloader configures delays per index * in the pinmux configuration, we pick which index to use for the pin On Beagleplay, for example, the HDMI hot plug detect GPIO benefited from this[1]. Corresponding pinctrl.h macros were posted in [2]. Why do it in the bootloader? since gpio inputs could also used in u-boot (e.g. MMC/CD) All said, Tom's question is very valid - we'd rather not modify evm.c for these specific configurations (popcorn as they might be), and we need to figure out a better option to introduce this kind of variation cleanly. For now, I will try dropping this patch. [1] https://git.beagleboard.org/beagleboard/linux/-/blob/v6.1.33-ti-rt-arm64-r6/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts#L311 [2] https://lore.kernel.org/all/20230619131620.3286650-1...@ti.com/ -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
Re: [PATCH 13/14] bloblist: Add alignment to bloblist_new()
I'm not sure why you would add an API to allow setting this explicitly, that's not really how it is supposed to work according to the spec. New TLs are always supposed to start with an alignment requirement of 8 (2^3) and then automatically increase that value if a TE gets added that has a larger requirement. There should be no use case where you'd want to initially create a TL that already has a larger number in that field (it doesn't make any difference... in particular, note that just because the alignment field has a larger value doesn't mean that the start of the TL must be aligned to that larger boundary; the field is only used to preserve the offset from the next alignment boundary during relocation).
Re: [PATCH 11/14] bloblist: Reduce blob-header size
> - rec->tag = tag;
> - rec->hdr_size = sizeof(struct bloblist_rec);
> + rec->tag_and_hdr_size = tag | sizeof(*rec) <<
> BLOBLISTR_HDR_SIZE_SHIFT;
nit: Might be a good idea to mask the tag or double-check that it fits
the field size here.
> - * 5. Bloblist uses 16-byte alignment internally and is designed to start on
> a
> - * 16-byte boundary. Its headers are multiples of 16 bytes. This makes it
> easier
> - * to deal with data structures which need this level of alignment, such as
> ACPI
> - * tables. For use in SPL and TPL the alignment can be relaxed, since it can
> be
> - * relocated to an aligned address in U-Boot proper.
> + * 5. Bloblist uses 8-byte alignment internally and is designed to start on a
> + * 8-byte boundary. Its headers are 8 bytes long. It is possible to achieve
> + * larger alignment (e.g. 16 bytes) by adding a dummy header, For use in SPL
> and
nit: I would call it a "dummy entry", it's not always just a header.
> + BLOBLIST_BLOB_ALIGN_LOG2 = 3,
> + BLOBLIST_BLOB_ALIGN = 1 << BLOBLIST_BLOB_ALIGN_LOG2,
> +
> BLOBLIST_ALIGN_LOG2 = 4,
> BLOBLIST_ALIGN = 1 << BLOBLIST_ALIGN_LOG2,
> };
> @@ -181,17 +184,25 @@ struct bloblist_hdr {
Note that there's no specific requirement for the TL header to be
aligned to 16 bytes. Even though it is 16 bytes long, 8 bytes
alignment is enough (so you shouldn't really need a BLOBLIST_ALIGN
that's different from BLOBLIST_BLOB_ALIGN).
There's also some text above this in the docstring for this struct
that refers to 16 bytes and should be updated. (I would recommend also
updating the "it can be safely moved around" part to point to the
instructions for relocation in the firmware_handoff spec, since while
it can be relocated, special care must be taken to preserve alignment
restrictions.
Re: [PATCH 10/14] bloblist: Handle alignment with a void entry
> /* Calculate the new allocated total */
> - new_alloced = data_start + ALIGN(size, 1U << align_log2);
> + new_alloced = data_start - map_to_sysmem(hdr) +
> + ALIGN(size, 1U << align_log2);
I think this is incorrect. There's no requirement that the size of an
entry must also be aligned as strictly as its start offset. So if
someone calls this code as bloblist_addrec(tag, 16, 8, ptr), then it
will try to create a blob at a 256 byte boundary with only 16 bytes of
data size, which is perfectly legal, but this code here will set
new_alloced as if the data size was also 256. That's not correct and
would likely throw off calculations elsewhere later. The alignment to
the start of the next entry is always just 8 bytes, so this line
should use BLOBLIST_BLOB_ALIGN_LOG2 (or sizeof(*rec)) instead of
align_log2.
> if (new_alloced > hdr->size) {
> log_err("Failed to allocate %x bytes size=%x, need size=%x\n",
> @@ -153,7 +168,7 @@ static int bloblist_addrec(uint tag, int size, int
> align_log2,
> rec = (void *)hdr + hdr->alloced;
>
> rec->tag = tag;
> - rec->hdr_size = data_start - hdr->alloced;
> + rec->hdr_size = sizeof(struct bloblist_rec);
> rec->size = size;
You also need to update the TL header alignment field if the requested
alignment here is greater, e.g. something like
if (hdr->alignment < align_log2)
hdr->alignment = align_log2;
Re: [PATCH 01/14] bloblist: Update the tag numbering
> diff --git a/include/bloblist.h b/include/bloblist.h > index 7ea72c6bd46..bad5fbbb889 100644 > --- a/include/bloblist.h > +++ b/include/bloblist.h nit: I would suggest also updating the documentation at the top of this file (point 7) to clarify that new standardized tags must be allocated in the firmware_handoff repo? Also, to point out the obvious, there are a bunch of tags in the standardized range in this file that don't match the firmware_handoff spec. I guess you could add them since 0x100 is still free. However, at least the BLOBLISTT_ACPI_TABLES tag would likely(?) be redundant with the existing XFERLIST_ACPI_AGGR tag. > + BLOBLISTT_U_BOOT_SPL_HANDOFF= 0xfff000, /* Hand-off info from SPL > */ > + BLOBLISTT_VBE = 0xfff001, /* VBE per-phase state */ > + BLOBLISTT_U_BOOT_VIDEO = 0xfff002, /* Video info from SPL */ FWIW, according to my view of the tag allocation philosophy, I think these kinds of tags should be allocated as official tags in the standardized range (e.g. in a cluster of U-Boot-specific tags). I would really recommend completely avoiding the non-standardized range for anything other than local experimentation or tags internal to closed-source code.
Re: [PATCH v5 06/12] Dockerfile: capsule: Setup the files needed for capsule update testing
On Thu, Jul 27, 2023 at 01:30:04AM +0530, Sughosh Ganu wrote:
> On Wed, 26 Jul 2023 at 22:09, Tom Rini wrote:
> >
> > On Wed, Jul 26, 2023 at 08:11:44PM +0530, Sughosh Ganu wrote:
> > > hi Simon,
> > >
> > > On Wed, 26 Jul 2023 at 19:41, Simon Glass wrote:
> > > >
> > > > Hi Tom,
> > > >
> > > > On Wed, 26 Jul 2023 at 07:23, Tom Rini wrote:
> > > > >
> > > > > On Wed, Jul 26, 2023 at 03:16:38PM +0530, Sughosh Ganu wrote:
> > > > > > On Wed, 26 Jul 2023 at 04:26, Tom Rini wrote:
> > > > > > >
> > > > > > > On Tue, Jul 25, 2023 at 04:52:38PM -0600, Simon Glass wrote:
> > > > > > > > On Tue, 25 Jul 2023 at 02:58, Sughosh Ganu
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Support has being added through earlier commits to build
> > > > > > > > > capsules
> > > > > > > > > and embed the public key needed for capsule authentication as
> > > > > > > > > part of
> > > > > > > > > u-boot build.
> > > > > > > > >
> > > > > > > > > From the testing point-of-view, this means the input files
> > > > > > > > > needed for
> > > > > > > > > generating the above have to be setup before invoking the
> > > > > > > > > build. Set
> > > > > > > > > this up in the CI configuration docker file for testing the
> > > > > > > > > capsule
> > > > > > > > > update feature.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Sughosh Ganu
> > > > > > > > > ---
> > > > > > > > > Changes since V4:
> > > > > > > > > * New patch which moves the setting up of the files needed
> > > > > > > > > for testing
> > > > > > > > > the EFI capsule update feature to the Dockerfile.
> > > > > > > > >
> > > > > > > > > Note: Earlier, this setup was being done in the azure and
> > > > > > > > > gitlab yaml
> > > > > > > > > files. Now that this has been moved to the Dockerfile, this
> > > > > > > > > will
> > > > > > > > > require generating a new container image and referencing that
> > > > > > > > > image in
> > > > > > > > > the yaml files for the CI to work when these patches get
> > > > > > > > > applied.
> > > > > > > > >
> > > > > > > > > tools/docker/Dockerfile | 12
> > > > > > > > > 1 file changed, 12 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
> > > > > > > > > index 3d2b64a355..294a0b0a53 100644
> > > > > > > > > --- a/tools/docker/Dockerfile
> > > > > > > > > +++ b/tools/docker/Dockerfile
> > > > > > > > > @@ -206,6 +206,18 @@ RUN mkdir -p /opt/nokia && \
> > > > > > > > > cp /tmp/qemu-linaro/arm-softmmu/qemu-system-arm
> > > > > > > > > /opt/nokia && \
> > > > > > > > > rm -rf /tmp/qemu-linaro
> > > > > > > > >
> > > > > > > > > +# Set up capsule files for UEFI capsule update testing
> > > > > > > > > +RUN mkdir -p /tmp/capsules && \
> > > > > > > > > +cd /tmp/capsules/ && \
> > > > > > > >
> > > > > > > > You can just use ${UBOOT_TRAVIS_BUILD_DIR} here
> > > > > > >
> > > > > > > That's not present in Dockerfiles, only at runtime within jobs
> > > > > > > (because
> > > > > > > we set it).
> > > > > >
> > > > > > I can copy the files into UBOOT_TRAVIS_BUILD_DIR as part of the job,
> > > > > > similar to what is being done for the grub*.efi files.
> > > > >
> > > > > Yes, copying the files rather than relying on them being in /tmp is
> > > > > better, but..
> > > > >
> > > > > > > > > +echo -n "u-boot:Old" > u-boot.bin.old && \
> > > > > > > > > +echo -n "u-boot:New" > u-boot.bin.new && \
> > > > > > > > > +echo -n "u-boot-env:Old" > u-boot.env.old && \
> > > > > > > > > +echo -n "u-boot-env:New" > u-boot.env.new && \
> > > > > > > >
> > > > > > > > We don't want these files, just the certs, since they are the
> > > > > > > > things
> > > > > > > > that take a long time:
> > > > > > > >
> > > > > > > > > +openssl req -x509 -sha256 -newkey rsa:2048 -subj
> > > > > > > > > /CN=TEST_SIGNER/ -keyout SIGNER.key -out SIGNER.crt -nodes
> > > > > > > > > -days 365 && \
> > > > > > > > > +openssl req -x509 -sha256 -newkey rsa:2048 -subj
> > > > > > > > > /CN=TEST_SIGNER/ -keyout SIGNER2.key -out SIGNER2.crt -nodes
> > > > > > > > > -days 365 && \
> > > > > > > > > +cert-to-efi-sig-list SIGNER.crt SIGNER.esl && \
> > > > > > > > > +chmod -R uog+rw /tmp/capsules/
> > > > > > >
> > > > > > > How long does it even take to make these certs? I'm not sure it's
> > > > > > > great
> > > > > > > to make these and stage them in /tmp and expect them to be around
> > > > > > > at
> > > > > > > test time.
> > > > > >
> > > > > > Should I mimic what is being done for the various grub.efi files? I
> > > > > > believe that these are in the /opt/grub/ directory of the docker
> > > > > > image, and get copied to the build dir at runtime.
> > > > >
> > > > > It takes 10 minutes or so to build grub, and we use it in multiple
> > > > > tests. Running openssl takes not even a second. Why are we doing this
> > > > > in the Dockerfile? Is this needed in more than one test? If so, does
> > > > > it
Re: [PATCH v5 06/12] Dockerfile: capsule: Setup the files needed for capsule update testing
On Wed, 26 Jul 2023 at 22:09, Tom Rini wrote:
>
> On Wed, Jul 26, 2023 at 08:11:44PM +0530, Sughosh Ganu wrote:
> > hi Simon,
> >
> > On Wed, 26 Jul 2023 at 19:41, Simon Glass wrote:
> > >
> > > Hi Tom,
> > >
> > > On Wed, 26 Jul 2023 at 07:23, Tom Rini wrote:
> > > >
> > > > On Wed, Jul 26, 2023 at 03:16:38PM +0530, Sughosh Ganu wrote:
> > > > > On Wed, 26 Jul 2023 at 04:26, Tom Rini wrote:
> > > > > >
> > > > > > On Tue, Jul 25, 2023 at 04:52:38PM -0600, Simon Glass wrote:
> > > > > > > On Tue, 25 Jul 2023 at 02:58, Sughosh Ganu
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Support has being added through earlier commits to build
> > > > > > > > capsules
> > > > > > > > and embed the public key needed for capsule authentication as
> > > > > > > > part of
> > > > > > > > u-boot build.
> > > > > > > >
> > > > > > > > From the testing point-of-view, this means the input files
> > > > > > > > needed for
> > > > > > > > generating the above have to be setup before invoking the
> > > > > > > > build. Set
> > > > > > > > this up in the CI configuration docker file for testing the
> > > > > > > > capsule
> > > > > > > > update feature.
> > > > > > > >
> > > > > > > > Signed-off-by: Sughosh Ganu
> > > > > > > > ---
> > > > > > > > Changes since V4:
> > > > > > > > * New patch which moves the setting up of the files needed for
> > > > > > > > testing
> > > > > > > > the EFI capsule update feature to the Dockerfile.
> > > > > > > >
> > > > > > > > Note: Earlier, this setup was being done in the azure and
> > > > > > > > gitlab yaml
> > > > > > > > files. Now that this has been moved to the Dockerfile, this will
> > > > > > > > require generating a new container image and referencing that
> > > > > > > > image in
> > > > > > > > the yaml files for the CI to work when these patches get
> > > > > > > > applied.
> > > > > > > >
> > > > > > > > tools/docker/Dockerfile | 12
> > > > > > > > 1 file changed, 12 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
> > > > > > > > index 3d2b64a355..294a0b0a53 100644
> > > > > > > > --- a/tools/docker/Dockerfile
> > > > > > > > +++ b/tools/docker/Dockerfile
> > > > > > > > @@ -206,6 +206,18 @@ RUN mkdir -p /opt/nokia && \
> > > > > > > > cp /tmp/qemu-linaro/arm-softmmu/qemu-system-arm
> > > > > > > > /opt/nokia && \
> > > > > > > > rm -rf /tmp/qemu-linaro
> > > > > > > >
> > > > > > > > +# Set up capsule files for UEFI capsule update testing
> > > > > > > > +RUN mkdir -p /tmp/capsules && \
> > > > > > > > +cd /tmp/capsules/ && \
> > > > > > >
> > > > > > > You can just use ${UBOOT_TRAVIS_BUILD_DIR} here
> > > > > >
> > > > > > That's not present in Dockerfiles, only at runtime within jobs
> > > > > > (because
> > > > > > we set it).
> > > > >
> > > > > I can copy the files into UBOOT_TRAVIS_BUILD_DIR as part of the job,
> > > > > similar to what is being done for the grub*.efi files.
> > > >
> > > > Yes, copying the files rather than relying on them being in /tmp is
> > > > better, but..
> > > >
> > > > > > > > +echo -n "u-boot:Old" > u-boot.bin.old && \
> > > > > > > > +echo -n "u-boot:New" > u-boot.bin.new && \
> > > > > > > > +echo -n "u-boot-env:Old" > u-boot.env.old && \
> > > > > > > > +echo -n "u-boot-env:New" > u-boot.env.new && \
> > > > > > >
> > > > > > > We don't want these files, just the certs, since they are the
> > > > > > > things
> > > > > > > that take a long time:
> > > > > > >
> > > > > > > > +openssl req -x509 -sha256 -newkey rsa:2048 -subj
> > > > > > > > /CN=TEST_SIGNER/ -keyout SIGNER.key -out SIGNER.crt -nodes
> > > > > > > > -days 365 && \
> > > > > > > > +openssl req -x509 -sha256 -newkey rsa:2048 -subj
> > > > > > > > /CN=TEST_SIGNER/ -keyout SIGNER2.key -out SIGNER2.crt -nodes
> > > > > > > > -days 365 && \
> > > > > > > > +cert-to-efi-sig-list SIGNER.crt SIGNER.esl && \
> > > > > > > > +chmod -R uog+rw /tmp/capsules/
> > > > > >
> > > > > > How long does it even take to make these certs? I'm not sure it's
> > > > > > great
> > > > > > to make these and stage them in /tmp and expect them to be around at
> > > > > > test time.
> > > > >
> > > > > Should I mimic what is being done for the various grub.efi files? I
> > > > > believe that these are in the /opt/grub/ directory of the docker
> > > > > image, and get copied to the build dir at runtime.
> > > >
> > > > It takes 10 minutes or so to build grub, and we use it in multiple
> > > > tests. Running openssl takes not even a second. Why are we doing this
> > > > in the Dockerfile? Is this needed in more than one test? If so, does it
> > > > matter if we have the same certs in each test?
> > >
> > > Yes it is actually much faster that I expected, so I suppose we can go
> > > back to having it in the test itself, e.g. in a pytest fixture.
> >
> > If not part of the docker image, these commands will still have to run
> > as part of the azure and g
Re: [PATCH v16 09/10] arm_ffa: efi: introduce FF-A MM communication
On Wed, Jul 26, 2023 at 10:45:02AM +0100, Abdellatif El Khlifi wrote: > Add MM communication support using FF-A transport > > This feature allows accessing MM partitions services through > EFI MM communication protocol. MM partitions such as StandAlonneMM > or smm-gateway secure partitions which reside in secure world. > > An MM shared buffer and a door bell event are used to exchange > the data. > > The data is used by EFI services such as GetVariable()/SetVariable() > and copied from the communication buffer to the MM shared buffer. > > The secure partition is notified about availability of data in the > MM shared buffer by an FF-A message (door bell). > > On such event, MM SP can read the data and updates the MM shared > buffer with the response data. > > The response data is copied back to the communication buffer and > consumed by the EFI subsystem. > > MM communication protocol supports FF-A 64-bit direct messaging. > > Signed-off-by: Abdellatif El Khlifi > Tested-by: Gowtham Suresh Kumar > Reviewed-by: Simon Glass > Cc: Tom Rini > Cc: Ilias Apalodimas > Cc: Jens Wiklander So, at this point in the series we impact lx2160ardb_tfa_stmm which is the only config in the tree prior to this series that sets CONFIG_EFI_MM_COMM_TEE. I'm not going to block this series[1] on updating lx2160ardb_tfa_stmm as well, but I do want to make sure the maintainers there are aware and can update the config to support the current state of this technology. [1]: https://patchwork.ozlabs.org/project/uboot/list/?series=365876&state=* -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 2/2] schemas: Add a schema for binman
Hi,
On Thu, 20 Jul 2023 at 13:56, Simon Glass wrote:
>
> With this version I have done with a generic name, in this case 'data',
> as suggested by Alper Nebi Yasak. This may be controversial, but we may
> as well have the dicussion now. I assume that there are no other
> ongoing attempts to define the layout of firmware in devicetree.
>
> Signed-off-by: Simon Glass
> ---
>
> Changes in v2:
> - Reworked significantly based on Alper's comments
>
> dtschema/schemas/firmware/binman/entry.yaml | 80 +
> dtschema/schemas/firmware/image.yaml| 77
> 2 files changed, 157 insertions(+)
> create mode 100644 dtschema/schemas/firmware/binman/entry.yaml
> create mode 100644 dtschema/schemas/firmware/image.yaml
>
> diff --git a/dtschema/schemas/firmware/binman/entry.yaml
> b/dtschema/schemas/firmware/binman/entry.yaml
> new file mode 100644
> index 000..d50f96d
> --- /dev/null
> +++ b/dtschema/schemas/firmware/binman/entry.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Google LLC
> +
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/image/entry.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Image entry
> +
> +maintainers:
> + - Simon Glass
> +
> +description: |
> + The entry node specifies a single entry in the firmware image.
> +
> + Entries have a specific type, such as "u-boot" or "atf-bl31". This is
> provided
> + using compatible = "data,".
> +
> + Note: This definition is intended to be hierarchical, so that entries can
> + appear in other entries. Schema for that is TBD.
> +
> +properties:
> + $nodename:
> +pattern: "^[-a-z]+(-[0-9]+)?$"
> +
> + compatible:
> +$ref: /schemas/types.yaml#/definitions/string
> +
> + offset:
> +$ref: /schemas/types.yaml#/definitions/uint32
> +description: |
> + Provides the offset of this entry from the start of its parent section.
> +
> + This may be omitted in the description provided by Binman, in which
> case
> + the value is calculated as part of image packing.
> +
> + size:
> +$ref: /schemas/types.yaml#/definitions/uint32
> +description: |
> + Provides the size of this entry in bytes.
> +
> + This may be omitted in the description provided by Binman, in which
> case
> + the value is calculated as part of image packing.
> +
> + reg:
> +description: |
> + Defines the offset and size of this entry, with reference to its parent
> + image / section.
> +
> + Note This is typically omitted in the description provided to Binman,
> + since the value is calculated as part of image packing. Separate
> + properties are provided for the size and offset of an entry, so that
> it is
> + easy to specify none, one or both. The `reg` property is the only one
> that
> + needs to be looked at once the image has been built.
> +
> +required:
> + - compatible
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +firmware {
> + image {
> +compatible = "data,image";
> +#address-cells = <1>;
> +$size-cells = <1>;
> +
> +u-boot@0 {
> + compatible = "data,u-boot";
> + reg = <0 0xa>;
> +};
> +
> +atf-bl31@0x10 {
> + compatible = "data,atf-bl31";
> + reg = <0x10 0x2>;
> +};
> + };
> +};
> diff --git a/dtschema/schemas/firmware/image.yaml
> b/dtschema/schemas/firmware/image.yaml
> new file mode 100644
> index 000..949b067
> --- /dev/null
> +++ b/dtschema/schemas/firmware/image.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Google LLC
> +
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/image.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Binman firmware layout
> +
> +maintainers:
> + - Simon Glass
> +
> +description: |
> + The image node provides a layout for firmware, used when packaging firmware
> + from multiple projects. For now it just supports a very simple set of
> + features, as a starting point for discussion.
> +
> + The Binman tool processes this node to produce a final image which can be
> + loaded into suitable storage device. Documentation is at:
> +
> + https://u-boot.readthedocs.io/en/latest/develop/package/binman.html
> +
> + The current image-description format is here:
> +
> +
> https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#image-description-format
> +
> + It is desirable to reference the image from the storage-device node,
> perhaps
> + using an image-desc property:
> +
> +spiflash@0 {
> + compatible = "spidev", "jedec,spi-nor";
> + image-desc = <&image>;
> +};
> +
> + Note that the intention is to change Binman to use whatever schema is
> agreed
> + here.
> +
> +properties:
> + $nodename:
> +const: binman
> +
> + compatib
Re: [PATCH V2 1/3] omap: timer: add ti,am654-timer compatibility
On 7/26/23 10:10 AM, Nishanth Menon wrote:
From: Sjoerd Simons
THe TI AM654 timer is compatible with the omap-timer implementation, so
s/THe/The
add it to the id list
s/id list/compatible ID list.
Andrew
Signed-off-by: Sjoerd Simons
Reviewed-by: Tom Rini
Tested-by: Ravi Gunasekaran
Tested-by: Mattijs Korpershoek
Cc: Francesco Dolcini
Cc: Wadim Egorov
Signed-off-by: Nishanth Menon
---
No Changes since V1.
V1: https://lore.kernel.org/all/20230725125856.1807742-2...@ti.com/
Original Patch:
https://lore.kernel.org/r/20230406185542.1179073-2-sjo...@collabora.com
drivers/timer/omap-timer.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/timer/omap-timer.c b/drivers/timer/omap-timer.c
index aa2e4360c1bb..9b6d97dae677 100644
--- a/drivers/timer/omap-timer.c
+++ b/drivers/timer/omap-timer.c
@@ -114,6 +114,7 @@ static const struct udevice_id omap_timer_ids[] = {
{ .compatible = "ti,am335x-timer" },
{ .compatible = "ti,am4372-timer" },
{ .compatible = "ti,omap5430-timer" },
+ { .compatible = "ti,am654-timer" },
{}
};
