Re: [RFC 01/22] part: call part_init() in blk_get_device_by_str() only for MMC
On 10/1/21 7:01 AM, AKASHI Takahiro wrote: In blk_get_device_by_str(), the comment says: "Updates the partition table for the specified hw partition." Since hw partition is supported only on MMC, it makes no sense to do so for other devices. Signed-off-by: AKASHI Takahiro --- disk/part.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/disk/part.c b/disk/part.c index a6a8f7052bd3..b330103a5bc0 100644 --- a/disk/part.c +++ b/disk/part.c @@ -427,7 +427,8 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str, * Always should be done, otherwise hw partition 0 will return stale * data after displaying a non-zero hw partition. */ - part_init(*dev_desc); + if ((*dev_desc)->if_type == IF_TYPE_MMC) + part_init(*dev_desc); For an eMMC the following logical levels exist: * device * hardware partition * software partition Linux might show the following: /dev/mmcblk0 - user data area /dev/mmcblk0boot0 - boot hardware partition 0 /dev/mmcblk0boot1 - boot hardware partition 1 /dev/mmcblk0rpmb - replay protected memory block How are the different hardware partition modeled in the UEFI device path? Should each hardware partition be a separate udevice? For NOR flash we also have an extra level: => setenv mtdparts mtdparts=30bb.qspi:1m(U-Boot),512k(Env),512k(DTB),2m(User_FS),12m(Data_FS),4m(Factory_FS),34m(Ramdisk),10m(Linux) => mtd device nor0 <30bb.qspi>, # parts = 8 #: namesizeoffset mask_flags 0: U-Boot 0x0010 0x 0 1: Env 0x0008 0x0010 0 2: DTB 0x0008 0x0018 0 3: User_FS 0x0020 0x0020 0 4: Data_FS 0x00c0 0x0040 0 5: Factory_FS 0x0040 0x0100 0 6: Ramdisk 0x0220 0x0140 0 7: Linux 0x00a0 0x0360 0 active partition: nor0,0 - (U-Boot) 0x0010 @ 0x Has part_info() to be called here too? What is the if_type? What are the devicepaths for these partitions? Best regards Heinrich #endif cleanup:
Re: [PATCH u-boot-marvell v3 39/39] MAINTAINERS: Add entry for kwbimage / kwboot tools
On 24.09.21 23:07, Marek Behún wrote: From: Marek Behún Add entry for these tools with Marek, Pali and Stefan as maintainers. Signed-off-by: Marek Behún Reviewed-by: Stefan Roese Thanks, Stefan --- MAINTAINERS | 10 ++ 1 file changed, 10 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 67c96a6045..6f103230da 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -776,6 +776,16 @@ S: Maintained T:git https://source.denx.de/u-boot/custodians/u-boot-i2c.git F:drivers/i2c/ +KWBIMAGE / KWBOOT TOOLS +M: Pali Rohár +M: Marek Behún +M: Stefan Roese +S: Maintained +T: git https://source.denx.de/u-boot/custodians/u-boot-marvell.git +F: doc/README.kwbimage +F: doc/kwboot.1 +F: tools/kwb* + LOGGING M:Simon Glass S:Maintained Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 38/39] doc/kwboot.1: Update man page
On 24.09.21 23:07, Marek Behún wrote: From: Marek Behún Update man page for the kwboot utility. Signed-off-by: Marek Behún Reviewed-by: Stefan Roese Thanks, Stefan --- doc/kwboot.1 | 60 ++-- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/doc/kwboot.1 b/doc/kwboot.1 index 1e9ca268f7..acdea891d9 100644 --- a/doc/kwboot.1 +++ b/doc/kwboot.1 @@ -1,21 +1,22 @@ -.TH KWBOOT 1 "2012-05-19" +.TH KWBOOT 1 "2021-08-25" .SH NAME -kwboot \- Boot Marvell Kirkwood SoCs over a serial link. +kwboot \- Boot Marvell Kirkwood (and others 32-bit) SoCs over a serial link. .SH SYNOPSIS .B kwboot .RB [ "-b \fIimage\fP" ] -.RB [ "-p" ] .RB [ "-t" ] .RB [ "-B \fIbaudrate\fP" ] .RB \fITTY\fP .SH "DESCRIPTION" -The \fBmkimage\fP program boots boards based on Marvell's Kirkwood -platform over their integrated UART. Boot image files will typically +The \fBkwboot\fP program boots boards based on Marvell's 32-bit +platforms including Kirkwood, Dove, A370, AXP, A375, A38x +and A39x over their integrated UART. Boot image files will typically contain a second stage boot loader, such as U-Boot. The image file must conform to Marvell's BootROM firmware image format -(\fIkwbimage\fP), created using a tool such as \fBmkimage\fP. +(\fIkwbimage v0\fP or \fIv1\fP), created using a tool such as +\fBmkimage\fP. Following power-up or a system reset, system BootROM code polls the UART for a brief period of time, sensing a handshake message which @@ -36,25 +37,23 @@ by the second-stage loader. Handshake; then upload file \fIimage\fP over \fITTY\fP. Note that for the encapsulated boot code to be executed, \fIimage\fP -must be of type "UART boot" (0x69). Boot images of different types, -such as backup images of vendor firmware downloaded from flash memory -(type 0x8B), will not work (or not as expected). See \fB-p\fP for a -workaround. +must be of type "UART boot" (0x69). The \fBkwboot\fP program changes +this type automatically, unless the \fIimage\fP is signed, in which +case it cannot be changed. This mode writes handshake status and upload progress indication to -stdout. +stdout. It is possible that \fIimage\fP contains an optional binary +code in it's header which may also print some output via UART (for +example U-Boot SPL does this). In such a case, this output is also +written to stdout after the header is sent. .TP .BI "\-p" -In combination with \fB-b\fP, patches the header in \fIimage\fP prior -to upload, to "UART boot" type. +Obsolete. Does nothing. -This option attempts on-the-fly conversion of some none-UART image -types, such as images which were originally formatted to be stored in -flash memory. - -Conversion is performed in memory. The contents of \fIimage\fP will -not be altered. +In the past, when this option was used, the program patched the header +in the image prior upload, to "UART boot" type. This is now done by +default. .TP .BI "\-t" @@ -65,11 +64,26 @@ If used in combination with \fB-b\fP, terminal mode is entered immediately following a successful image upload. If standard I/O streams connect to a console, this mode will terminate -after receiving 'ctrl-\\' followed by 'c' from console input. +after receiving \fBctrl-\e\fP followed by \fBc\fP from console input. .TP .BI "\-B \fIbaudrate\fP" -Adjust the baud rate on \fITTY\fP. Default rate is 115200. +If used in combination with \fB-b\fP, inject into the image header +code that changes baud rate to \fIbaudrate\fP after uploading image +header, and code that changes the baud rate back to the default +(115200 Bd) before executing payload, and also adjust the baud rate +on \fITTY\fP correspondingly. This can make the upload significantly +faster. + +If used in combination with \fB-t\fP, adjust the baud rate to +\fIbaudrate\fP on \fITTY\fP before starting terminal. + +If both \fB-b\fP and \fB-t\fP are used, the baud rate is changed +back to 115200 after the upload. + +Tested values for \fIbaudrate\fP for Armada 38x include: 115200, +230400, 460800, 50, 576000, 921600, 100, 1152000, 150, +200, 250, 3125000, 400 and 520. .SH "SEE ALSO" .PP @@ -82,3 +96,7 @@ Daniel Stodden Luka Perkov .br David Purdy +.br +Pali Rohár +.br +Marek Behún Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 37/39] tools: kwboot: Add Pali and Marek as authors
On 24.09.21 23:07, Marek Behún wrote: From: Pali Rohár Add Pali and Marek as another authors of the kwboot utility. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Reviewed-by: Stefan Roese Thanks, Stefan --- tools/kwboot.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/kwboot.c b/tools/kwboot.c index 6fa6dff04d..6a1a030308 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -4,6 +4,8 @@ * Armada 39x * * (c) 2012 Daniel Stodden + * (c) 2021 Pali Rohár + * (c) 2021 Marek Behún * * References: marvell.com, "88F6180, 88F6190, 88F6192, and 88F6281 * Integrated Controller: Functional Specifications" December 2, Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 36/39] tools: kwboot: Update file header
On 24.09.21 23:07, Marek Behún wrote: From: Marek Behún Mention all supported platforms in file header. Signed-off-by: Marek Behún Reviewed-by: Stefan Roese Thanks, Stefan --- tools/kwboot.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index 9da5b42ebf..6fa6dff04d 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1,6 +1,7 @@ /* * Boot a Marvell SoC, with Xmodem over UART0. - * supports Kirkwood, Dove, Armada 370, Armada XP + * supports Kirkwood, Dove, Armada 370, Armada XP, Armada 375, Armada 38x and + * Armada 39x * * (c) 2012 Daniel Stodden * Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 35/39] tools: kwboot: Avoid code repetition in kwboot_img_patch()
On 24.09.21 23:07, Marek Behún wrote:
From: Marek Behún
Change kwboot_img_patch() to avoid code repetition of setting errno to
EINVAL.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 55 ++
1 file changed, 20 insertions(+), 35 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 9eb007f1bb..9da5b42ebf 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1386,7 +1386,6 @@ _copy_baudrate_change_code(struct main_hdr_v1 *hdr, void
*dst, int pre,
static int
kwboot_img_patch(void *img, size_t *size, int baudrate)
{
- int rc;
struct main_hdr_v1 *hdr;
uint32_t srcaddr;
uint8_t csum;
@@ -1394,33 +1393,25 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
int image_ver;
int is_secure;
- rc = -1;
hdr = img;
- if (*size < sizeof(struct main_hdr_v1)) {
- errno = EINVAL;
- goto out;
- }
+ if (*size < sizeof(struct main_hdr_v1))
+ goto err;
image_ver = kwbimage_version(img);
if (image_ver != 0 && image_ver != 1) {
fprintf(stderr, "Invalid image header version\n");
- errno = EINVAL;
- goto out;
+ goto err;
}
hdrsz = kwbheader_size(hdr);
- if (*size < hdrsz) {
- errno = EINVAL;
- goto out;
- }
+ if (*size < hdrsz)
+ goto err;
csum = kwboot_hdr_csum8(hdr) - hdr->checksum;
- if (csum != hdr->checksum) {
- errno = EINVAL;
- goto out;
- }
+ if (csum != hdr->checksum)
+ goto err;
if (image_ver == 0) {
struct main_hdr_v0 *hdr_v0 = img;
@@ -1433,10 +1424,9 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
switch (hdr->blockid) {
case IBR_HDR_SATA_ID:
- if (srcaddr < 1) {
- errno = EINVAL;
- goto out;
- }
+ if (srcaddr < 1)
+ goto err;
+
hdr->srcaddr = cpu_to_le32((srcaddr - 1) * 512);
break;
@@ -1459,10 +1449,8 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
}
if (hdrsz > le32_to_cpu(hdr->srcaddr) ||
- *size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize)) {
- errno = EINVAL;
- goto out;
- }
+ *size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize))
+ goto err;
is_secure = kwboot_img_is_secure(img);
@@ -1470,8 +1458,7 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
if (is_secure) {
fprintf(stderr,
"Image has secure header with signature for non-UART
booting\n");
- errno = EINVAL;
- goto out;
+ goto err;
}
kwboot_printv("Patching image boot signature to UART\n");
@@ -1485,15 +1472,13 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
if (image_ver == 0) {
fprintf(stderr,
"Cannot inject code for changing baudrate into v0
image header\n");
- errno = EINVAL;
- goto out;
+ goto err;
}
if (is_secure) {
fprintf(stderr,
"Cannot inject code for changing baudrate into image
with secure header\n");
- errno = EINVAL;
- goto out;
+ goto err;
}
/*
@@ -1529,8 +1514,7 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
if (is_secure) {
fprintf(stderr, "Cannot align image with secure
header\n");
- errno = EINVAL;
- goto out;
+ goto err;
}
kwboot_printv("Aligning image header to Xmodem block size\n");
@@ -1540,9 +1524,10 @@ kwboot_img_patch(void *img, size_t *size, int baudrate)
hdr->checksum = kwboot_hdr_csum8(hdr) - csum;
*size = le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize);
- rc = 0;
-out:
- return rc;
+ return 0;
+err:
+ errno = EINVAL;
+ return -1;
}
static void
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 34/39] tools: kwboot: Cosmetic fix
On 24.09.21 23:07, Marek Behún wrote:
From: Marek Behún
Add spaces around the | operator.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 5e491f31a4..9eb007f1bb 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -644,7 +644,7 @@ kwboot_open_tty(const char *path, int baudrate)
rc = -1;
- fd = open(path, O_RDWR|O_NOCTTY|O_NDELAY);
+ fd = open(path, O_RDWR | O_NOCTTY | O_NDELAY);
if (fd < 0)
goto out;
@@ -653,7 +653,7 @@ kwboot_open_tty(const char *path, int baudrate)
goto out;
cfmakeraw(&tio);
- tio.c_cflag |= CREAD|CLOCAL;
+ tio.c_cflag |= CREAD | CLOCAL;
tio.c_cc[VMIN] = 1;
tio.c_cc[VTIME] = 0;
@@ -1137,7 +1137,7 @@ kwboot_terminal(int tty)
}
kwboot_printv("[Type Ctrl-%c + %c to quit]\r\n",
- quit[0]|0100, quit[1]);
+ quit[0] | 0100, quit[1]);
} else
in = -1;
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 32/39] tools: kwboot: Disable tty interbyte timeout
On 24.09.21 23:07, Marek Behún wrote: From: Pali Rohár Function kwboot_tty_recv() has its own handling of read timeout, we don't need to do set it in tty settings. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Reviewed-by: Stefan Roese Thanks, Stefan --- tools/kwboot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index fc83161d70..a527c79cf3 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -655,7 +655,7 @@ kwboot_open_tty(const char *path, int baudrate) cfmakeraw(&tio); tio.c_cflag |= CREAD|CLOCAL; tio.c_cc[VMIN] = 1; - tio.c_cc[VTIME] = 10; + tio.c_cc[VTIME] = 0; rc = tcsetattr(fd, TCSANOW, &tio); if (rc) Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 31/39] tools: kwboot: Fix initializing tty device
On 24.09.21 23:07, Marek Behún wrote: From: Pali Rohár Retrieve current terminal settings via tcgetattr(), set to raw mode with cfmakeraw(), enable receiver via CREAD and ignore modem control lines via CLOCAL. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Reviewed-by: Stefan Roese Thanks, Stefan --- tools/kwboot.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index d8b950787b..fc83161d70 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -648,11 +648,12 @@ kwboot_open_tty(const char *path, int baudrate) if (fd < 0) goto out; - memset(&tio, 0, sizeof(tio)); - - tio.c_iflag = 0; - tio.c_cflag = CREAD|CLOCAL|CS8; + rc = tcgetattr(fd, &tio); + if (rc) + goto out; + cfmakeraw(&tio); + tio.c_cflag |= CREAD|CLOCAL; tio.c_cc[VMIN] = 1; tio.c_cc[VTIME] = 10; Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 30/39] tools: kwboot: Check whether baudrate was set to requested value
On 24.09.21 23:07, Marek Behún wrote:
From: Marek Behún
The tcsetattr() function can return 0 even if baudrate was not changed.
Check whether baudrate was changed to requested value, and in case of
arbitrary baudrate, check whether the set value is within 3% tolerance.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 32
1 file changed, 32 insertions(+)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 7ccab2993f..d8b950787b 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -567,6 +567,13 @@ kwboot_tty_baudrate_to_speed(int baudrate)
}
}
+static int
+_is_within_tolerance(int value, int reference, int tolerance)
+{
+ return 100 * value >= reference * (100 - tolerance) &&
+ 100 * value <= reference * (100 + tolerance);
+}
+
static int
kwboot_tty_change_baudrate(int fd, int baudrate)
{
@@ -601,7 +608,32 @@ kwboot_tty_change_baudrate(int fd, int baudrate)
if (rc)
return rc;
+ rc = tcgetattr(fd, &tio);
+ if (rc)
+ return rc;
+
+ if (cfgetospeed(&tio) != speed || cfgetispeed(&tio) != speed)
+ goto baud_fail;
+
+#ifdef BOTHER
+ /*
+* Check whether set baudrate is within 3% tolerance.
+* If BOTHER is defined, Linux always fills out c_ospeed / c_ispeed
+* with real values.
+*/
+ if (!_is_within_tolerance(tio.c_ospeed, baudrate, 3))
+ goto baud_fail;
+
+ if (!_is_within_tolerance(tio.c_ispeed, baudrate, 3))
+ goto baud_fail;
+#endif
+
return 0;
+
+baud_fail:
+ fprintf(stderr, "Could not set baudrate to requested value\n");
+ errno = EINVAL;
+ return -1;
}
static int
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 29/39] tools: kwboot: Allow any baudrate on Linux
On 24.09.21 23:07, Marek Behún wrote:
From: Pali Rohár
The A38x platform supports more baudrates than just those defined by the
Bn constants, and some of them are higher than the highest Bn baudrate
(the highest is 4 MBd while A38x support 5.15 MBd).
On Linux, add support for arbitrary baudrates. (Since there is no
standard POSIX API to specify arbitrary baudrate for a tty device, this
change is Linux-specific.)
We need to use raw TCGETS2/TCSETS2 or TCGETS/TCSETS ioctls with the
BOTHER flag in struct termios2/termios, defined in Linux headers
(included by ) and . Since
these headers conflict with glibc's header file , it is not
possible to use libc's termios functions and we need to reimplement them
via ioctl() calls.
Note that the Bnnn constants from need not be compatible
with Bnnn constants from .
Signed-off-by: Pali Rohár
[ termios macros rewritten to static inline functions (for type control)
and moved to tools/termios_linux.h ]
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c| 16 +++-
tools/termios_linux.h | 189 ++
2 files changed, 204 insertions(+), 1 deletion(-)
create mode 100644 tools/termios_linux.h
diff --git a/tools/kwboot.c b/tools/kwboot.c
index ba2fd10ff6..7ccab2993f 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -23,10 +23,15 @@
#include
#include
#include
-#include
#include
#include
+#ifdef __linux__
+#include "termios_linux.h"
+#else
+#include
+#endif
+
/*
* Marvell BootROM UART Sensing
*/
@@ -554,7 +559,11 @@ kwboot_tty_baudrate_to_speed(int baudrate)
return B50;
#endif
default:
+#ifdef BOTHER
+ return BOTHER;
+#else
return B0;
+#endif
}
}
@@ -575,6 +584,11 @@ kwboot_tty_change_baudrate(int fd, int baudrate)
return -1;
}
+#ifdef BOTHER
+ if (speed == BOTHER)
+ tio.c_ospeed = tio.c_ispeed = baudrate;
+#endif
+
rc = cfsetospeed(&tio, speed);
if (rc)
return rc;
diff --git a/tools/termios_linux.h b/tools/termios_linux.h
new file mode 100644
index 00..d73989b625
--- /dev/null
+++ b/tools/termios_linux.h
@@ -0,0 +1,189 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * termios fuctions to support arbitrary baudrates (on Linux)
+ *
+ * Copyright (c) 2021 Pali Rohár
+ * Copyright (c) 2021 Marek Behún
+ */
+
+#ifndef _TERMIOS_LINUX_H_
+#define _TERMIOS_LINUX_H_
+
+/*
+ * We need to use raw TCGETS2/TCSETS2 or TCGETS/TCSETS ioctls with the BOTHER
+ * flag in struct termios2/termios, defined in Linux headers
+ * (included by ) and . Since these headers
+ * conflict with glibc's header file , it is not possible to use
+ * libc's termios functions and we need to reimplement them via ioctl() calls.
+ *
+ * An arbitrary baudrate is supported when the macro BOTHER is defined. The
+ * baudrate value itself is then stored into the c_ospeed and c_ispeed members.
+ * If ioctls TCGETS2/TCSETS2 are defined and supported then these fields are
+ * present in struct termios2, otherwise these fields are present in struct
+ * termios.
+ *
+ * Note that the Bnnn constants from need not be compatible with
Bnnn
+ * constants from .
+ */
+
+#include
+#include
+#include
+#include
+
+#if defined(BOTHER) && defined(TCGETS2)
+#define termios termios2
+#endif
+
+static inline int tcgetattr(int fd, struct termios *t)
+{
+#if defined(BOTHER) && defined(TCGETS2)
+ return ioctl(fd, TCGETS2, t);
+#else
+ return ioctl(fd, TCGETS, t);
+#endif
+}
+
+static inline int tcsetattr(int fd, int a, const struct termios *t)
+{
+ int cmd;
+
+ switch (a) {
+#if defined(BOTHER) && defined(TCGETS2)
+ case TCSANOW:
+ cmd = TCSETS2;
+ break;
+ case TCSADRAIN:
+ cmd = TCSETSW2;
+ break;
+ case TCSAFLUSH:
+ cmd = TCSETSF2;
+ break;
+#else
+ case TCSANOW:
+ cmd = TCSETS;
+ break;
+ case TCSADRAIN:
+ cmd = TCSETSW;
+ break;
+ case TCSAFLUSH:
+ cmd = TCSETSF;
+ break;
+#endif
+ default:
+ errno = EINVAL;
+ return -1;
+ }
+
+ return ioctl(fd, cmd, t);
+}
+
+static inline int tcdrain(int fd)
+{
+ return ioctl(fd, TCSBRK, 1);
+}
+
+static inline int tcflush(int fd, int q)
+{
+ return ioctl(fd, TCFLSH, q);
+}
+
+static inline int tcsendbreak(int fd, int d)
+{
+ return ioctl(fd, TCSBRK, d);
+}
+
+static inline int tcflow(int fd, int a)
+{
+ return ioctl(fd, TCXONC, a);
+}
+
+static inline pid_t tcgetsid(int fd)
+{
+ pid_t sid;
+
+ if (ioctl(fd, TIOCGSID, &sid) < 0)
+ return (pid_t)-1;
+
+ return sid;
+}
+
+static inline speed_t cfgetospeed(const struct termios *t)
+{
+ return t->c_cflag & CBAUD;
+}
+
+static inli
Re: [PATCH u-boot-marvell v3 28/39] tools: kwboot: Support higher baudrates when booting via UART
On 24.09.21 23:07, Marek Behún wrote:
From: Pali Rohár
Add support for uploading the boot image (the data part only) at higher
baudrate than the standard one.
The kwboot utility already has -B option, but choosing other baudrate
than the standard one (115200 Bd) can only work for debug mode, not for
booting the device. The BootROM for kwboot supported platforms (Orion,
Kirkwood, Dove, Discovery, AXP, A37x, A38x, A39x) cannot change the
baudrate when uploading boot image via the Xmodem protocol, nor can it
be configured via strapping pins.
So instead we add this support by injecting baudrate changing code into
the kwbimage v1 header as a new optional binary extension. This code is
executed by BootROM after it receives the whole header. The code sends
the magic string "$baudratechange\0" just before changing the baudrate
to let kwboot know that it should also change it. This is because the
injected code is run as the last binary extension, and we do not want
to loose possible output from other possible binary extensions that
came before it (in most cases this is U-Boot SPL).
We also inject the code before the payload (the data part of the image),
to change the baudrate back to the standard value, in case the payload
does not reset UART.
This change improves boot time via UART significantly (depending on the
chosen baudrate), which is very useful when debugging.
Signed-off-by: Pali Rohár
[ major refactor ]
Signed-off-by: Marek Behún
Impressive change.
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 637 ++---
1 file changed, 603 insertions(+), 34 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 77bf5cb80b..ba2fd10ff6 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -70,6 +70,187 @@ struct kwboot_block {
#define KWBOOT_BLK_RSP_TIMEO 1000 /* ms */
#define KWBOOT_HDR_RSP_TIMEO 1 /* ms */
+/* ARM code making baudrate changing function return to original exec address */
+static unsigned char kwboot_pre_baud_code[] = {
+ /* exec_addr: */
+ 0x00, 0x00, 0x00, 0x00, /* .word 0*/
+ 0x0c, 0xe0, 0x1f, 0xe5, /* ldr lr, exec_addr */
+};
+
+/* ARM code for binary header injection to change baudrate */
+static unsigned char kwboot_baud_code[] = {
+ /* ; #define UART_BASE 0xd0012000 */
+ /* ; #define THR 0x00 */
+ /* ; #define DLL 0x00 */
+ /* ; #define DLH 0x04 */
+ /* ; #define LCR 0x0c */
+ /* ; #define DLAB0x80 */
+ /* ; #define LSR 0x14 */
+ /* ; #define THRE0x20 */
+ /* ; #define TEMT0x40 */
+ /* ; #define DIV_ROUND(a, b) ((a + b/2) / b) */
+ /* ; */
+ /* ; u32 set_baudrate(u32 old_b, u32 new_b) { */
+ /* ; const u8 *str = "$baudratechange"; */
+ /* ; u8 c; */
+ /* ; do { */
+ /* ; c = *str++;*/
+ /* ; writel(UART_BASE + THR, c);*/
+ /* ; } while (c); */
+ /* ; while */
+ /* ; (!(readl(UART_BASE + LSR) & TEMT)); */
+ /* ; u32 lcr = readl(UART_BASE + LCR); */
+ /* ; writel(UART_BASE + LCR, lcr | DLAB); */
+ /* ; u8 old_dll = readl(UART_BASE + DLL); */
+ /* ; u8 old_dlh = readl(UART_BASE + DLH); */
+ /* ; u16 old_dl = old_dll | (old_dlh << 8); */
+ /* ; u32 clk = old_b * old_dl; */
+ /* ; u16 new_dl = DIV_ROUND(clk, new_b);*/
+ /* ; u8 new_dll = new_dl & 0xff;*/
+ /* ; u8 new_dlh = (new_dl >> 8) & 0xff; */
+ /* ; writel(UART_BASE + DLL, new_dll); */
+ /* ; writel(UART_BASE + DLH, new_dlh); */
+ /* ; writel(UART_BASE + LCR, lcr & ~DLAB);
Re: [PATCH u-boot-marvell v3 27/39] tools: kwboot: Explicitly check against size of struct main_hdr_v1
On 24.09.21 23:07, Marek Behún wrote:
From: Marek Behún
Explicitly check the image size against size of struct main_hdr_v1.
This way the check is more readable, since the `hdrsz` variable
may semantically contain another value.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 4fae44c46e..77bf5cb80b 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -820,14 +820,14 @@ kwboot_img_patch_hdr(void *img, size_t *size)
struct main_hdr_v1 *hdr;
uint32_t srcaddr;
uint8_t csum;
- size_t hdrsz = sizeof(*hdr);
+ size_t hdrsz;
int image_ver;
int is_secure;
rc = -1;
hdr = img;
- if (*size < hdrsz) {
+ if (*size < sizeof(struct main_hdr_v1)) {
errno = EINVAL;
goto out;
}
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 26/39] tools: kwboot: Round up header size to 128 B when patching
On 24.09.21 23:07, Marek Behún wrote:
From: Pali Rohár
The beginning of image data must be sent in a separate xmodem block;
the block must not contain end of header with the beginning of data.
Therefore we need to ensure that the image header size is a multiple of
xmodem block size (which is 128 B).
Read the file into a malloc()ed buffer of enough size instead of
mmap()ing it. (If we are going to move the data, most of the pages will
be dirty anyway.) Then move the payload if header size needs to be
increased.
Signed-off-by: Pali Rohár
[ refactored ]
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 89 +++---
1 file changed, 77 insertions(+), 12 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index e60f7c5b7a..4fae44c46e 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -25,7 +25,6 @@
#include
#include
#include
-#include
#include
/*
@@ -706,11 +705,12 @@ out:
}
static void *
-kwboot_mmap_image(const char *path, size_t *size)
+kwboot_read_image(const char *path, size_t *size, size_t reserve)
{
int rc, fd;
struct stat st;
void *img;
+ off_t tot;
rc = -1;
img = NULL;
@@ -723,17 +723,30 @@ kwboot_mmap_image(const char *path, size_t *size)
if (rc)
goto out;
- img = mmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
- if (img == MAP_FAILED) {
- img = NULL;
+ img = malloc(st.st_size + reserve);
+ if (!img)
goto out;
+
+ tot = 0;
+ while (tot < st.st_size) {
+ ssize_t rd = read(fd, img + tot, st.st_size - tot);
+
+ if (rd < 0)
+ goto out;
+
+ tot += rd;
+
+ if (!rd && tot < st.st_size) {
+ errno = EIO;
+ goto out;
+ }
}
rc = 0;
*size = st.st_size;
out:
if (rc && img) {
- munmap(img, st.st_size);
+ free(img);
img = NULL;
}
if (fd >= 0)
@@ -769,8 +782,39 @@ kwboot_img_is_secure(void *img)
return 0;
}
+static void
+kwboot_img_grow_hdr(void *img, size_t *size, size_t grow)
+{
+ uint32_t hdrsz, datasz, srcaddr;
+ struct main_hdr_v1 *hdr = img;
+ uint8_t *data;
+
+ srcaddr = le32_to_cpu(hdr->srcaddr);
+
+ hdrsz = kwbheader_size(img);
+ data = (uint8_t *)img + srcaddr;
+ datasz = *size - srcaddr;
+
+ /* only move data if there is not enough space */
+ if (hdrsz + grow > srcaddr) {
+ size_t need = hdrsz + grow - srcaddr;
+
+ /* move data by enough bytes */
+ memmove(data + need, data, datasz);
+
+ hdr->srcaddr = cpu_to_le32(srcaddr + need);
+ *size += need;
+ }
+
+ if (kwbimage_version(img) == 1) {
+ hdrsz += grow;
+ hdr->headersz_msb = hdrsz >> 16;
+ hdr->headersz_lsb = cpu_to_le16(hdrsz & 0x);
+ }
+}
+
static int
-kwboot_img_patch_hdr(void *img, size_t size)
+kwboot_img_patch_hdr(void *img, size_t *size)
{
int rc;
struct main_hdr_v1 *hdr;
@@ -783,7 +827,7 @@ kwboot_img_patch_hdr(void *img, size_t size)
rc = -1;
hdr = img;
- if (size < hdrsz) {
+ if (*size < hdrsz) {
errno = EINVAL;
goto out;
}
@@ -797,7 +841,7 @@ kwboot_img_patch_hdr(void *img, size_t size)
hdrsz = kwbheader_size(hdr);
- if (size < hdrsz) {
+ if (*size < hdrsz) {
errno = EINVAL;
goto out;
}
@@ -844,6 +888,12 @@ kwboot_img_patch_hdr(void *img, size_t size)
break;
}
+ if (hdrsz > le32_to_cpu(hdr->srcaddr) ||
+ *size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize)) {
+ errno = EINVAL;
+ goto out;
+ }
+
is_secure = kwboot_img_is_secure(img);
if (hdr->blockid != IBR_HDR_UART_ID) {
@@ -858,8 +908,23 @@ kwboot_img_patch_hdr(void *img, size_t size)
hdr->blockid = IBR_HDR_UART_ID;
}
+ if (hdrsz % KWBOOT_XM_BLKSZ) {
+ size_t offset = (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) %
+ KWBOOT_XM_BLKSZ;
+
+ if (is_secure) {
+ fprintf(stderr, "Cannot align image with secure
header\n");
+ errno = EINVAL;
+ goto out;
+ }
+
+ kwboot_printv("Aligning image header to Xmodem block size\n");
+ kwboot_img_grow_hdr(img, size, offset);
+ }
+
hdr->checksum = kwboot_hdr_csum8(hdr) - csum;
+ *size = le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize);
rc = 0;
out:
return rc;
@@ -986,13 +1051,13 @@ main(int
Re: [PATCH u-boot-marvell v3 25/39] tools: kwbimage: Update comments describing kwbimage v1 structures
On 24.09.21 23:07, Marek Behún wrote:
From: Pali Rohár
These structures are relevant for several other platforms, mention them
all.
Signed-off-by: Pali Rohár
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwbimage.c | 3 ++-
tools/kwbimage.h | 6 +++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index d1f4f93e0f..77bf4dd886 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -1,7 +1,8 @@
// SPDX-License-Identifier: GPL-2.0+
/*
* Image manipulator for Marvell SoCs
- * supports Kirkwood, Dove, Armada 370, Armada XP, and Armada 38x
+ * supports Kirkwood, Dove, Armada 370, Armada XP, Armada 375, Armada 38x and
+ * Armada 39x
*
* (C) Copyright 2013 Thomas Petazzoni
*
diff --git a/tools/kwbimage.h b/tools/kwbimage.h
index 56a748d4cf..679c454367 100644
--- a/tools/kwbimage.h
+++ b/tools/kwbimage.h
@@ -69,7 +69,7 @@ struct ext_hdr_v0 {
uint8_t checksum;
} __packed;
-/* Structure of the main header, version 1 (Armada 370/38x/XP) */
+/* Structure of the main header, version 1 (Armada 370/XP/375/38x/39x) */
struct main_hdr_v1 {
uint8_t blockid; /* 0x0 */
uint8_t flags; /* 0x1 */
@@ -103,7 +103,7 @@ struct main_hdr_v1 {
#define MAIN_HDR_V1_OPT_BAUD_115200 0x7
/*
- * Header for the optional headers, version 1 (Armada 370, Armada XP)
+ * Header for the optional headers, version 1 (Armada 370/XP/375/38x/39x)
*/
struct opt_hdr_v1 {
uint8_t headertype;
@@ -127,7 +127,7 @@ struct sig_v1 {
} __packed;
/*
- * Structure of secure header (Armada 38x)
+ * Structure of secure header (Armada XP/375/38x/39x)
*/
struct secure_hdr_v1 {
uint8_t headertype;/* 0x0 */
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 24/39] tools: kwbimage: Refactor kwbimage header size determination
On 24.09.21 23:07, Marek Behún wrote:
From: Marek Behún
Add functions kwbheader_size() and kwbheader_size_for_csum().
Refactor code determining header size to use these functions.
Refactor header checksum determining function.
Remove stuff that is not needed anymore.
This simplifies the code a little and fixes one instance of validating
header size meant for checksum instead of whole header size.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwbimage.c | 29 +++--
tools/kwbimage.h | 35 +++
tools/kwboot.c | 22 ++
3 files changed, 40 insertions(+), 46 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 944a108cee..d1f4f93e0f 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -280,14 +280,6 @@ static uint8_t image_checksum8(void *start, uint32_t len)
return csum;
}
-size_t kwbimage_header_size(unsigned char *ptr)
-{
- if (kwbimage_version((void *)ptr) == 0)
- return sizeof(struct main_hdr_v0);
- else
- return KWBHEADER_V1_SIZE((struct main_hdr_v1 *)ptr);
-}
-
/*
* Verify checksum over a complete header that includes the checksum field.
* Return 1 when OK, otherwise 0.
@@ -298,7 +290,7 @@ static int main_hdr_checksum_ok(void *hdr)
struct main_hdr_v0 *main_hdr = (struct main_hdr_v0 *)hdr;
uint8_t checksum;
- checksum = image_checksum8(hdr, kwbimage_header_size(hdr));
+ checksum = image_checksum8(hdr, kwbheader_size_for_csum(hdr));
/* Calculated checksum includes the header checksum field. Compensate
* for that.
*/
@@ -1649,8 +1641,8 @@ static int kwbimage_check_image_types(uint8_t type)
static int kwbimage_verify_header(unsigned char *ptr, int image_size,
struct image_tool_params *params)
{
- uint8_t checksum;
- size_t header_size = kwbimage_header_size(ptr);
+ size_t header_size = kwbheader_size(ptr);
+ uint8_t csum;
if (header_size > image_size)
return -FDT_ERR_BADSTRUCTURE;
@@ -1663,17 +1655,10 @@ static int kwbimage_verify_header(unsigned char *ptr,
int image_size,
struct main_hdr_v0 *mhdr = (struct main_hdr_v0 *)ptr;
if (mhdr->ext & 0x1) {
- struct ext_hdr_v0 *ext_hdr;
-
- if (header_size + sizeof(*ext_hdr) > image_size)
- return -FDT_ERR_BADSTRUCTURE;
+ struct ext_hdr_v0 *ext_hdr = (void *)(mhdr + 1);
- ext_hdr = (struct ext_hdr_v0 *)
- (ptr + sizeof(struct main_hdr_v0));
- checksum = image_checksum8(ext_hdr,
- sizeof(struct ext_hdr_v0)
- - sizeof(uint8_t));
- if (checksum != ext_hdr->checksum)
+ csum = image_checksum8(ext_hdr, sizeof(*ext_hdr) - 1);
+ if (csum != ext_hdr->checksum)
return -FDT_ERR_BADSTRUCTURE;
}
} else if (kwbimage_version(ptr) == 1) {
@@ -1832,7 +1817,7 @@ static int kwbimage_generate(struct image_tool_params
*params,
static int kwbimage_extract_subimage(void *ptr, struct image_tool_params
*params)
{
struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr;
- size_t header_size = kwbimage_header_size(ptr);
+ size_t header_size = kwbheader_size(ptr);
struct opt_hdr_v1 *ohdr;
int idx = params->pflag;
int cur_idx = 0;
diff --git a/tools/kwbimage.h b/tools/kwbimage.h
index 738034beb1..56a748d4cf 100644
--- a/tools/kwbimage.h
+++ b/tools/kwbimage.h
@@ -69,11 +69,6 @@ struct ext_hdr_v0 {
uint8_t checksum;
} __packed;
-struct kwb_header {
- struct main_hdr_v0 kwb_hdr;
- struct ext_hdr_v0 kwb_exthdr;
-} __packed;
-
/* Structure of the main header, version 1 (Armada 370/38x/XP) */
struct main_hdr_v1 {
uint8_t blockid; /* 0x0 */
@@ -195,13 +190,6 @@ struct register_set_hdr_v1 {
#define OPT_HDR_V1_BINARY_TYPE 0x2
#define OPT_HDR_V1_REGISTER_TYPE 0x3
-#define KWBHEADER_V0_SIZE(hdr) \
- (((hdr)->ext & 0x1) ? sizeof(struct kwb_header) : \
- sizeof(struct main_hdr_v0))
-
-#define KWBHEADER_V1_SIZE(hdr) \
- (((hdr)->headersz_msb << 16) | le16_to_cpu((hdr)->headersz_lsb))
-
enum kwbimage_cmd {
CMD_INVALID,
CMD_BOOT_FROM,
@@ -235,6 +223,29 @@ static inline unsigned int kwbimage_version(const void
*header)
return ptr[8];
}
+static inline size_t kwbheader_size(const void *header)
+{
+ if (kwbimage_version(header) == 0) {
+ const struct main_hdr_v0 *hdr = header;
+
+ return sizeof(*hdr) +
+ (hdr->ext & 0x
Re: [PATCH u-boot-marvell v3 23/39] tools: kwbimage: Refactor image_version()
On 24.09.21 23:07, Marek Behún wrote:
From: Marek Behún
Rename this function to kwbimage_version() and don't cast argument if
not needed.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwbimage.c | 8
tools/kwbimage.h | 4 ++--
tools/kwboot.c | 4 ++--
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 16b082b35f..944a108cee 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -282,7 +282,7 @@ static uint8_t image_checksum8(void *start, uint32_t len)
size_t kwbimage_header_size(unsigned char *ptr)
{
- if (image_version((void *)ptr) == 0)
+ if (kwbimage_version((void *)ptr) == 0)
return sizeof(struct main_hdr_v0);
else
return KWBHEADER_V1_SIZE((struct main_hdr_v1 *)ptr);
@@ -1622,7 +1622,7 @@ static void kwbimage_print_header(const void *ptr)
printf("Image Type: MVEBU Boot from %s Image\n",
image_boot_mode_name(mhdr->blockid));
- printf("Image version:%d\n", image_version((void *)ptr));
+ printf("Image version:%d\n", kwbimage_version(ptr));
for_each_opt_hdr_v1 (ohdr, mhdr) {
if (ohdr->headertype == OPT_HDR_V1_BINARY_TYPE) {
@@ -1659,7 +1659,7 @@ static int kwbimage_verify_header(unsigned char *ptr, int
image_size,
return -FDT_ERR_BADSTRUCTURE;
/* Only version 0 extended header has checksum */
- if (image_version((void *)ptr) == 0) {
+ if (kwbimage_version(ptr) == 0) {
struct main_hdr_v0 *mhdr = (struct main_hdr_v0 *)ptr;
if (mhdr->ext & 0x1) {
@@ -1676,7 +1676,7 @@ static int kwbimage_verify_header(unsigned char *ptr, int
image_size,
if (checksum != ext_hdr->checksum)
return -FDT_ERR_BADSTRUCTURE;
}
- } else if (image_version((void *)ptr) == 1) {
+ } else if (kwbimage_version(ptr) == 1) {
struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr;
const uint8_t *mhdr_end;
struct opt_hdr_v1 *ohdr;
diff --git a/tools/kwbimage.h b/tools/kwbimage.h
index 9a949e03c0..738034beb1 100644
--- a/tools/kwbimage.h
+++ b/tools/kwbimage.h
@@ -229,7 +229,7 @@ void init_kwb_image_type (void);
* header, byte 8 was reserved, and always set to 0. In the v1 header,
* byte 8 has been changed to a proper field, set to 1.
*/
-static inline unsigned int image_version(const void *header)
+static inline unsigned int kwbimage_version(const void *header)
{
const unsigned char *ptr = header;
return ptr[8];
@@ -258,7 +258,7 @@ static inline int opt_hdr_v1_valid_size(const struct
opt_hdr_v1 *ohdr,
static inline struct opt_hdr_v1 *opt_hdr_v1_first(void *img) {
struct main_hdr_v1 *mhdr;
- if (image_version(img) != 1)
+ if (kwbimage_version(img) != 1)
return NULL;
mhdr = img;
diff --git a/tools/kwboot.c b/tools/kwboot.c
index b1dcd3796c..e47bf94e89 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -583,7 +583,7 @@ kwboot_xmodem(int tty, const void *_img, size_t size)
int rc, pnum;
size_t hdrsz;
- if (image_version(img) == 0)
+ if (kwbimage_version(img) == 0)
hdrsz = KWBHEADER_V0_SIZE((struct main_hdr_v0 *)img);
else
hdrsz = KWBHEADER_V1_SIZE((struct main_hdr_v1 *)img);
@@ -787,7 +787,7 @@ kwboot_img_patch_hdr(void *img, size_t size)
goto out;
}
- image_ver = image_version(img);
+ image_ver = kwbimage_version(img);
if (image_ver != 0 && image_ver != 1) {
fprintf(stderr, "Invalid image header version\n");
errno = EINVAL;
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 22/39] tools: kwboot: Patch destination address to DDR area for SPI image
On 24.09.21 23:06, Marek Behún wrote:
From: Pali Rohár
SPI/NOR kwbimage may have destination address set to 0x, which
means that the image is not downloaded to DDR but rather it is executed
directly from SPI/NOR. In this case execution address is set to SPI/NOR
area.
When patching image to UART type, change destination and execution
addresses from SPI/NOR XIP area to DDR area 0x0080 (which is default
for A38x).
Signed-off-by: Pali Rohár
[ refactored ]
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 8
1 file changed, 8 insertions(+)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 907a574bfc..b1dcd3796c 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -836,6 +836,14 @@ kwboot_img_patch_hdr(void *img, size_t size)
if (srcaddr == 0x)
hdr->srcaddr = cpu_to_le32(hdrsz);
break;
+
+ case IBR_HDR_SPI_ID:
+ if (hdr->destaddr == cpu_to_le32(0x)) {
+ kwboot_printv("Patching destination and execution addresses
from SPI/NOR XIP area to DDR area 0x0080\n");
+ hdr->destaddr = cpu_to_le32(0x0080);
+ hdr->execaddr = cpu_to_le32(0x0080);
+ }
+ break;
}
is_secure = kwboot_img_is_secure(img);
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 21/39] tools: kwboot: Patch source address in image header
On 24.09.21 23:06, Marek Behún wrote:
From: Pali Rohár
Some image types have source address in non-bytes unit; for example for
SATA images, it is in 512 B units.
We need to multiply by unit size when patching image type to UART.
Signed-off-by: Pali Rohár
[ refactored ]
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 40 +---
1 file changed, 29 insertions(+), 11 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 2446d0a7b5..907a574bfc 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -773,6 +773,7 @@ kwboot_img_patch_hdr(void *img, size_t size)
{
int rc;
struct main_hdr_v1 *hdr;
+ uint32_t srcaddr;
uint8_t csum;
size_t hdrsz = sizeof(*hdr);
int image_ver;
@@ -809,6 +810,34 @@ kwboot_img_patch_hdr(void *img, size_t size)
goto out;
}
+ if (image_ver == 0) {
+ struct main_hdr_v0 *hdr_v0 = img;
+
+ hdr_v0->nandeccmode = IBR_HDR_ECC_DISABLED;
+ hdr_v0->nandpagesize = 0;
+ }
+
+ srcaddr = le32_to_cpu(hdr->srcaddr);
+
+ switch (hdr->blockid) {
+ case IBR_HDR_SATA_ID:
+ if (srcaddr < 1) {
+ errno = EINVAL;
+ goto out;
+ }
+ hdr->srcaddr = cpu_to_le32((srcaddr - 1) * 512);
+ break;
+
+ case IBR_HDR_SDIO_ID:
+ hdr->srcaddr = cpu_to_le32(srcaddr * 512);
+ break;
+
+ case IBR_HDR_PEX_ID:
+ if (srcaddr == 0x)
+ hdr->srcaddr = cpu_to_le32(hdrsz);
+ break;
+ }
+
is_secure = kwboot_img_is_secure(img);
if (hdr->blockid != IBR_HDR_UART_ID) {
@@ -823,17 +852,6 @@ kwboot_img_patch_hdr(void *img, size_t size)
hdr->blockid = IBR_HDR_UART_ID;
}
- if (image_ver == 0) {
- struct main_hdr_v0 *hdr_v0 = img;
-
- hdr_v0->nandeccmode = IBR_HDR_ECC_DISABLED;
- hdr_v0->nandpagesize = 0;
-
- hdr_v0->srcaddr = hdr_v0->ext
- ? sizeof(struct kwb_header)
- : sizeof(*hdr_v0);
- }
-
hdr->checksum = kwboot_img_csum8(hdr, hdrsz) - csum;
rc = 0;
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 18/39] tools: kwboot: Always call kwboot_img_patch_hdr()
On 24.09.21 23:06, Marek Behún wrote:
From: Pali Rohár
The kwboot_img_patch_hdr() function already decides if header patching
is needed. Always call this function and deprecate the unneeded command
line option `-p`.
Signed-off-by: Pali Rohár
Reviewed-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 23 ++-
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index ad91afd075..9394a51380 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -709,9 +709,9 @@ out:
}
static void *
-kwboot_mmap_image(const char *path, size_t *size, int prot)
+kwboot_mmap_image(const char *path, size_t *size)
{
- int rc, fd, flags;
+ int rc, fd;
struct stat st;
void *img;
@@ -726,9 +726,7 @@ kwboot_mmap_image(const char *path, size_t *size, int prot)
if (rc)
goto out;
- flags = (prot & PROT_WRITE) ? MAP_PRIVATE : MAP_SHARED;
-
- img = mmap(NULL, st.st_size, prot, flags, fd, 0);
+ img = mmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd,
0);
if (img == MAP_FAILED) {
img = NULL;
goto out;
@@ -833,7 +831,6 @@ kwboot_usage(FILE *stream, char *progname)
fprintf(stream, "\n");
fprintf(stream,
" -b : boot with preamble (Kirkwood, Armada
370/XP)\n");
- fprintf(stream, " -p: patch to type 0x69 (uart boot)\n");
fprintf(stream,
" -D : boot without preamble (Dove)\n");
fprintf(stream, " -d: enter debug mode\n");
@@ -853,7 +850,7 @@ int
main(int argc, char **argv)
{
const char *ttypath, *imgpath;
- int rv, rc, tty, term, prot, patch;
+ int rv, rc, tty, term;
void *bootmsg;
void *debugmsg;
void *img;
@@ -867,7 +864,6 @@ main(int argc, char **argv)
imgpath = NULL;
img = NULL;
term = 0;
- patch = 0;
size = 0;
speed = B115200;
@@ -894,7 +890,7 @@ main(int argc, char **argv)
break;
case 'p':
- patch = 1;
+ /* nop, for backward compatibility */
break;
case 't':
@@ -934,9 +930,6 @@ main(int argc, char **argv)
if (!bootmsg && !term && !debugmsg)
goto usage;
- if (patch && !imgpath)
- goto usage;
-
if (argc - optind < 1)
goto usage;
@@ -949,16 +942,12 @@ main(int argc, char **argv)
}
if (imgpath) {
- prot = PROT_READ | (patch ? PROT_WRITE : 0);
-
- img = kwboot_mmap_image(imgpath, &size, prot);
+ img = kwboot_mmap_image(imgpath, &size);
if (!img) {
perror(imgpath);
goto out;
}
- }
- if (patch) {
rc = kwboot_img_patch_hdr(img, size);
if (rc) {
fprintf(stderr, "%s: Invalid image.\n", imgpath);
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 17/39] tools: kwboot: Properly finish xmodem transfer
On 24.09.21 23:06, Marek Behún wrote:
From: Pali Rohár
After kwboot sends EOT, BootROM sends back ACK. Add code for handling
this and retry sending EOT on error.
Signed-off-by: Pali Rohár
[ refactored ]
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 62 --
1 file changed, 45 insertions(+), 17 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 8c11dca5ed..ad91afd075 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -403,6 +403,29 @@ _is_xm_reply(char c)
return c == ACK || c == NAK || c == CAN;
}
+static int
+_xm_reply_to_error(int c)
+{
+ int rc = -1;
+
+ switch (c) {
+ case ACK:
+ rc = 0;
+ break;
+ case NAK:
+ errno = EBADMSG;
+ break;
+ case CAN:
+ errno = ECANCELED;
+ break;
+ default:
+ errno = EPROTO;
+ break;
+ }
+
+ return rc;
+}
+
static int
kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm, int *non_xm_print)
{
@@ -483,24 +506,29 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block,
int allow_non_xm,
if (non_xm_print)
kwboot_printv("\n");
- rc = -1;
+ return _xm_reply_to_error(c);
+}
- switch (c) {
- case ACK:
- rc = 0;
- break;
- case NAK:
- errno = EBADMSG;
- break;
- case CAN:
- errno = ECANCELED;
- break;
- default:
- errno = EPROTO;
- break;
- }
+static int
+kwboot_xm_finish(int fd)
+{
+ int rc, retries;
+ char c;
- return rc;
+ kwboot_printv("Finishing transfer\n");
+
+ retries = 16;
+ do {
+ rc = kwboot_tty_send_char(fd, EOT);
+ if (rc)
+ return rc;
+
+ rc = kwboot_xm_recv_reply(fd, &c, 0, NULL);
+ if (rc)
+ return rc;
+ } while (c == NAK && retries-- > 0);
+
+ return _xm_reply_to_error(c);
}
static int
@@ -577,7 +605,7 @@ kwboot_xmodem(int tty, const void *_img, size_t size)
if (rc)
return rc;
- return kwboot_tty_send_char(tty, EOT);
+ return kwboot_xm_finish(tty);
}
static int
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 16/39] tools: kwboot: Prevent waiting indefinitely if no xmodem reply is received
On 24.09.21 23:06, Marek Behún wrote:
From: Marek Behún
Currently if BootROM fails to respond with ACK/NAK to a xmodem block, we
will be waiting indefinitely for such response.
Make sure that we only wait at most 1 second (blk_rsp_timeo) for ACK/NAK
for each block in case non-xmodem text output is not being expected.
Interpret this timeout expiration as NAK, to try to send the block
again.
On the other hand, if timeout expires without ACK while some non-xmodem
output was already received (DDR training output, for example), we know
that the block was received, since the code is being executed, so in
this case exit with ETIMEDOUT.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index cf6e32c6fa..8c11dca5ed 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -407,17 +407,18 @@ static int
kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm, int *non_xm_print)
{
int timeout = allow_non_xm ? KWBOOT_HDR_RSP_TIMEO : blk_rsp_timeo;
- uint64_t recv_until = 0;
+ uint64_t recv_until = _now() + timeout;
int rc;
- *non_xm_print = 0;
+ if (non_xm_print)
+ *non_xm_print = 0;
while (1) {
rc = kwboot_tty_recv(fd, c, 1, timeout);
if (rc) {
if (errno != ETIMEDOUT)
return rc;
- else if (recv_until && recv_until < _now())
+ else if (allow_non_xm && *non_xm_print)
return -1;
else
*c = NAK;
@@ -430,12 +431,19 @@ kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm,
int *non_xm_print)
/*
* If printing non-xmodem text output is allowed and such a byte
* was received, print it and increase receiving time.
+* Otherwise decrease timeout by time elapsed.
*/
if (allow_non_xm) {
recv_until = _now() + timeout;
putchar(*c);
fflush(stdout);
*non_xm_print = 1;
+ } else {
+ timeout = recv_until - _now();
+ if (timeout < 0) {
+ errno = ETIMEDOUT;
+ return -1;
+ }
}
}
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 15/39] tools: kwboot: Allow greater timeout when executing header code
On 24.09.21 23:06, Marek Behún wrote:
From: Marek Behún
When executing header code (which contains U-Boot SPL in most cases),
wait 10s after every non-xmodem character received (i.e. printed by
U-Boot SPL) before timing out.
Sometimes DDR training, which runs in SPL, may be slow.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 34 +++---
1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 2f4c61bed6..cf6e32c6fa 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
@@ -68,6 +69,7 @@ struct kwboot_block {
} __packed;
#define KWBOOT_BLK_RSP_TIMEO 1000 /* ms */
+#define KWBOOT_HDR_RSP_TIMEO 1 /* ms */
static int kwboot_verbose;
@@ -375,6 +377,26 @@ kwboot_xm_makeblock(struct kwboot_block *block, const void *data,
return n;
}
+static uint64_t
+_now(void)
+{
+ struct timespec ts;
+
+ if (clock_gettime(CLOCK_MONOTONIC, &ts)) {
+ static int err_print;
+
+ if (!err_print) {
+ perror("clock_gettime() does not work");
+ err_print = 1;
+ }
+
+ /* this will just make the timeout not work */
+ return -1ULL;
+ }
+
+ return ts.tv_sec * 1000ULL + (ts.tv_nsec + 50) / 100;
+}
+
static int
_is_xm_reply(char c)
{
@@ -384,16 +406,21 @@ _is_xm_reply(char c)
static int
kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm, int *non_xm_print)
{
+ int timeout = allow_non_xm ? KWBOOT_HDR_RSP_TIMEO : blk_rsp_timeo;
+ uint64_t recv_until = 0;
int rc;
*non_xm_print = 0;
while (1) {
- rc = kwboot_tty_recv(fd, c, 1, blk_rsp_timeo);
+ rc = kwboot_tty_recv(fd, c, 1, timeout);
if (rc) {
if (errno != ETIMEDOUT)
return rc;
- *c = NAK;
+ else if (recv_until && recv_until < _now())
+ return -1;
+ else
+ *c = NAK;
}
/* If received xmodem reply, end. */
@@ -402,9 +429,10 @@ kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm,
int *non_xm_print)
/*
* If printing non-xmodem text output is allowed and such a byte
-* was received, print it.
+* was received, print it and increase receiving time.
*/
if (allow_non_xm) {
+ recv_until = _now() + timeout;
putchar(*c);
fflush(stdout);
*non_xm_print = 1;
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 14/39] tools: kwboot: Print new line after SPL output
On 24.09.21 23:06, Marek Behún wrote:
From: Marek Behún
There is no separation between output from the code from binary header
(U-Boot SPL in most cases) and subsequent kwboot output.
Print '\n' to make distinguishing these two easier.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 4636622a6c..2f4c61bed6 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -382,10 +382,12 @@ _is_xm_reply(char c)
}
static int
-kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm)
+kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm, int *non_xm_print)
{
int rc;
+ *non_xm_print = 0;
+
while (1) {
rc = kwboot_tty_recv(fd, c, 1, blk_rsp_timeo);
if (rc) {
@@ -405,6 +407,7 @@ kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm)
if (allow_non_xm) {
putchar(*c);
fflush(stdout);
+ *non_xm_print = 1;
}
}
@@ -415,6 +418,7 @@ static int
kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
int *done_print)
{
+ int non_xm_print;
int rc, retries;
char c;
@@ -432,7 +436,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
*done_print = 1;
}
- rc = kwboot_xm_recv_reply(fd, &c, allow_non_xm);
+ rc = kwboot_xm_recv_reply(fd, &c, allow_non_xm, &non_xm_print);
if (rc)
return rc;
@@ -440,6 +444,9 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
kwboot_progress(-1, '+');
} while (c == NAK && retries-- > 0);
+ if (non_xm_print)
+ kwboot_printv("\n");
+
rc = -1;
switch (c) {
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 13/39] tools: kwboot: Allow non-xmodem text output from BootROM only in a specific case
On 24.09.21 23:06, Marek Behún wrote:
From: Pali Rohár
When sending image header / image data, BootROM does not send any
non-xmodem text output. We should therefore interpret unknown bytes in
the xmodem protocol as errors and resend current packet. This should
improve the transfer in case there are errors on the UART line.
Text output from BootROM may only happen after whole image header is
sent and before ACK for the last packet of image header is received.
In this case BootROM may execute code from the image, which may interact
with UART (U-Boot SPL, for example, prints stuff on UART).
Print received non-xmodem output from BootROM only in this case.
Signed-off-by: Pali Rohár
[ refactored & simplified ]
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 70 ++
1 file changed, 53 insertions(+), 17 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 2e5684b91c..4636622a6c 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -382,33 +382,62 @@ _is_xm_reply(char c)
}
static int
-kwboot_xm_sendblock(int fd, struct kwboot_block *block)
+kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm)
+{
+ int rc;
+
+ while (1) {
+ rc = kwboot_tty_recv(fd, c, 1, blk_rsp_timeo);
+ if (rc) {
+ if (errno != ETIMEDOUT)
+ return rc;
+ *c = NAK;
+ }
+
+ /* If received xmodem reply, end. */
+ if (_is_xm_reply(*c))
+ break;
+
+ /*
+* If printing non-xmodem text output is allowed and such a byte
+* was received, print it.
+*/
+ if (allow_non_xm) {
+ putchar(*c);
+ fflush(stdout);
+ }
+ }
+
+ return 0;
+}
+
+static int
+kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm,
+ int *done_print)
{
int rc, retries;
char c;
+ *done_print = 0;
+
retries = 16;
do {
rc = kwboot_tty_send(fd, block, sizeof(*block));
if (rc)
return rc;
- do {
- rc = kwboot_tty_recv(fd, &c, 1, blk_rsp_timeo);
- if (rc) {
- if (errno != ETIMEDOUT)
- return rc;
- c = NAK;
- }
-
- if (!_is_xm_reply(c))
- printf("%c", c);
+ if (allow_non_xm && !*done_print) {
+ kwboot_progress(100, '.');
+ kwboot_printv("Done\n");
+ *done_print = 1;
+ }
- } while (!_is_xm_reply(c));
+ rc = kwboot_xm_recv_reply(fd, &c, allow_non_xm);
+ if (rc)
+ return rc;
- if (c != ACK)
+ if (!allow_non_xm && c != ACK)
kwboot_progress(-1, '+');
-
} while (c == NAK && retries-- > 0);
rc = -1;
@@ -435,6 +464,7 @@ static int
kwboot_xmodem_one(int tty, int *pnum, int header, const uint8_t *data,
size_t size)
{
+ int done_print = 0;
size_t sent, left;
int rc;
@@ -446,22 +476,28 @@ kwboot_xmodem_one(int tty, int *pnum, int header, const uint8_t *data,
while (sent < size) {
struct kwboot_block block;
+ int last_block;
size_t blksz;
blksz = kwboot_xm_makeblock(&block, data, left, (*pnum)++);
data += blksz;
- rc = kwboot_xm_sendblock(tty, &block);
+ last_block = (left <= blksz);
+
+ rc = kwboot_xm_sendblock(tty, &block, header && last_block,
+&done_print);
if (rc)
goto out;
sent += blksz;
left -= blksz;
- kwboot_progress(sent * 100 / size, '.');
+ if (!done_print)
+ kwboot_progress(sent * 100 / size, '.');
}
- kwboot_printv("Done\n");
+ if (!done_print)
+ kwboot_printv("Done\n");
return 0;
out:
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 12/39] tools: kwboot: Use a function to check whether received byte is a Xmodem reply
On 24.09.21 23:06, Marek Behún wrote:
From: Marek Behún
This is a non-functional change that should make the code more readable.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 7f231c0823..2e5684b91c 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -375,6 +375,12 @@ kwboot_xm_makeblock(struct kwboot_block *block, const void
*data,
return n;
}
+static int
+_is_xm_reply(char c)
+{
+ return c == ACK || c == NAK || c == CAN;
+}
+
static int
kwboot_xm_sendblock(int fd, struct kwboot_block *block)
{
@@ -395,10 +401,10 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block)
c = NAK;
}
- if (c != ACK && c != NAK && c != CAN)
+ if (!_is_xm_reply(c))
printf("%c", c);
- } while (c != ACK && c != NAK && c != CAN);
+ } while (!_is_xm_reply(c));
if (c != ACK)
kwboot_progress(-1, '+');
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 11/39] tools: kwboot: Split sending image into header and data stages
On 24.09.21 23:06, Marek Behún wrote:
From: Pali Rohár
This change is required to implement other features in kwboot.
Split sending header and data parts of the image into two stages.
Signed-off-by: Pali Rohár
[ refactored ]
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwbimage.h | 8 +++--
tools/kwboot.c | 84 +++-
2 files changed, 61 insertions(+), 31 deletions(-)
diff --git a/tools/kwbimage.h b/tools/kwbimage.h
index e063e3e41e..074bdfbe46 100644
--- a/tools/kwbimage.h
+++ b/tools/kwbimage.h
@@ -195,6 +195,10 @@ struct register_set_hdr_v1 {
#define OPT_HDR_V1_BINARY_TYPE 0x2
#define OPT_HDR_V1_REGISTER_TYPE 0x3
+#define KWBHEADER_V0_SIZE(hdr) \
+ (((hdr)->ext & 0x1) ? sizeof(struct kwb_header) : \
+ sizeof(struct main_hdr_v0))
+
#define KWBHEADER_V1_SIZE(hdr) \
(((hdr)->headersz_msb << 16) | le16_to_cpu((hdr)->headersz_lsb))
@@ -225,9 +229,9 @@ void init_kwb_image_type (void);
* header, byte 8 was reserved, and always set to 0. In the v1 header,
* byte 8 has been changed to a proper field, set to 1.
*/
-static inline unsigned int image_version(void *header)
+static inline unsigned int image_version(const void *header)
{
- unsigned char *ptr = header;
+ const unsigned char *ptr = header;
return ptr[8];
}
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 0e533e3698..7f231c0823 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -57,11 +57,13 @@ static unsigned char kwboot_msg_debug[] = {
#define NAK 21 /* target block negative ack */
#define CAN 24 /* target/sender transfer cancellation */
+#define KWBOOT_XM_BLKSZ 128 /* xmodem block size */
+
struct kwboot_block {
uint8_t soh;
uint8_t pnum;
uint8_t _pnum;
- uint8_t data[128];
+ uint8_t data[KWBOOT_XM_BLKSZ];
uint8_t csum;
} __packed;
@@ -356,16 +358,15 @@ static size_t
kwboot_xm_makeblock(struct kwboot_block *block, const void *data,
size_t size, int pnum)
{
- const size_t blksz = sizeof(block->data);
size_t i, n;
block->soh = SOH;
block->pnum = pnum;
block->_pnum = ~block->pnum;
- n = size < blksz ? size : blksz;
+ n = size < KWBOOT_XM_BLKSZ ? size : KWBOOT_XM_BLKSZ;
memcpy(&block->data[0], data, n);
- memset(&block->data[n], 0, blksz - n);
+ memset(&block->data[n], 0, KWBOOT_XM_BLKSZ - n);
block->csum = 0;
for (i = 0; i < n; i++)
@@ -425,48 +426,73 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block)
}
static int
-kwboot_xmodem(int tty, const void *_data, size_t size)
+kwboot_xmodem_one(int tty, int *pnum, int header, const uint8_t *data,
+ size_t size)
{
- const uint8_t *data = _data;
- int rc, pnum, N, err;
-
- pnum = 1;
- N = 0;
+ size_t sent, left;
+ int rc;
- kwboot_printv("Sending boot image...\n");
+ kwboot_printv("Sending boot image %s (%zu bytes)...\n",
+ header ? "header" : "data", size);
- sleep(2); /* flush isn't effective without it */
- tcflush(tty, TCIOFLUSH);
+ left = size;
+ sent = 0;
- do {
+ while (sent < size) {
struct kwboot_block block;
- int n;
+ size_t blksz;
- n = kwboot_xm_makeblock(&block,
- data + N, size - N,
- pnum++);
- if (!n)
- break;
+ blksz = kwboot_xm_makeblock(&block, data, left, (*pnum)++);
+ data += blksz;
rc = kwboot_xm_sendblock(tty, &block);
if (rc)
goto out;
- N += n;
- kwboot_progress(N * 100 / size, '.');
- } while (1);
+ sent += blksz;
+ left -= blksz;
+
+ kwboot_progress(sent * 100 / size, '.');
+ }
- rc = kwboot_tty_send_char(tty, EOT);
+ kwboot_printv("Done\n");
+ return 0;
out:
kwboot_printv("\n");
return rc;
+}
-can:
- err = errno;
- kwboot_tty_send_char(tty, CAN);
- errno = err;
- goto out;
+static int
+kwboot_xmodem(int tty, const void *_img, size_t size)
+{
+ const uint8_t *img = _img;
+ int rc, pnum;
+ size_t hdrsz;
+
+ if (image_version(img) == 0)
+ hdrsz = KWBHEADER_V0_SIZE((struct main_hdr_v0 *)img);
+ else
+ hdrsz = KWBHEADER_V1_SIZE((struct main_hdr_v1 *)img);
+
+ kwboot_printv("Waiting 2s and flushing tty\n");
+ sleep(2); /* flush isn't effective without it */
+ tcflush(tty, TCIOFLUSH);
+
+ pnum = 1;
+
+ rc = kwboot_xmodem_one(tty, &pnum, 1, img, hdrsz);
+ if (rc)
+ return rc;
+
+ img += hdrsz;
+ size -= hdrsz;
+
+
Re: [PATCH u-boot-marvell v3 10/39] tools: kwboot: Print newline on error when progress was not completed
On 24.09.21 23:06, Marek Behún wrote:
From: Pali Rohár
When progress was not completed, current terminal position is in progress
bar. So print newline before printing error message to make error message
more readable.
Signed-off-by: Pali Rohár
Reviewed-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index eb4b3fe230..0e533e3698 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -459,6 +459,7 @@ kwboot_xmodem(int tty, const void *_data, size_t size)
rc = kwboot_tty_send_char(tty, EOT);
out:
+ kwboot_printv("\n");
return rc;
can:
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 09/39] tools: kwboot: Fix printing progress
On 24.09.21 23:06, Marek Behún wrote:
From: Pali Rohár
Ensure that `pos` is still in range up to the `width` so printing 100%
works also for bigger images. After printing 100% progress reset it to
zero, so that next progressbar can be started.
Signed-off-by: Pali Rohár
Reviewed-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 3d9f73e697..eb4b3fe230 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -140,12 +140,14 @@ __progress(int pct, char c)
fputc(c, stdout);
nl = "]\n";
- pos++;
+ pos = (pos + 1) % width;
if (pct == 100) {
- while (pos++ < width)
+ while (pos && pos++ < width)
fputc(' ', stdout);
fputs(nl, stdout);
+ nl = "";
+ pos = 0;
}
fflush(stdout);
@@ -162,6 +164,9 @@ kwboot_progress(int _pct, char c)
if (kwboot_verbose)
__progress(pct, c);
+
+ if (pct == 100)
+ pct = 0;
}
static int
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 08/39] tools: kwboot: Fix comparison of integers with different size
On 24.09.21 23:06, Marek Behún wrote:
From: Marek Behún
The compiler complains that we are comparing int with size_t when
compiled with -W.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 88353d19c0..3d9f73e697 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -352,8 +352,7 @@ kwboot_xm_makeblock(struct kwboot_block *block, const void
*data,
size_t size, int pnum)
{
const size_t blksz = sizeof(block->data);
- size_t n;
- int i;
+ size_t i, n;
block->soh = SOH;
block->pnum = pnum;
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 07/39] tools: kwboot: Fix return type of kwboot_xm_makeblock() function
On 24.09.21 23:06, Marek Behún wrote:
From: Pali Rohár
Function kwboot_xm_makeblock() always returns length of xmodem block. It
is always non-negative and calculated from variable with size_t type. Set
return type of this function to size_t and remove dead code which checks
for negative value.
Signed-off-by: Pali Rohár
Reviewed-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index b9a402ca91..88353d19c0 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -347,7 +347,7 @@ kwboot_debugmsg(int tty, void *msg)
return rc;
}
-static int
+static size_t
kwboot_xm_makeblock(struct kwboot_block *block, const void *data,
size_t size, int pnum)
{
@@ -441,9 +441,6 @@ kwboot_xmodem(int tty, const void *_data, size_t size)
n = kwboot_xm_makeblock(&block,
data + N, size - N,
pnum++);
- if (n < 0)
- goto can;
-
if (!n)
break;
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 06/39] tools: kwboot: Fix kwboot_xm_sendblock() function when kwboot_tty_recv() fails
On 24.09.21 23:06, Marek Behún wrote:
From: Pali Rohár
When kwboot_tty_recv() fails or times out, it does not set the `c`
variable to NAK. The variable is then compared, while it holds either
an undefined value or a value from previous iteration. Set `c` to NAK so
that the other side will try to resend current block, and remove the
now unnecessary break.
In other failure cases return immediately.
Signed-off-by: Pali Rohár
Reviewed-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 454339db14..b9a402ca91 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -380,12 +380,15 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block)
do {
rc = kwboot_tty_send(fd, block, sizeof(*block));
if (rc)
- break;
+ return rc;
do {
rc = kwboot_tty_recv(fd, &c, 1, blk_rsp_timeo);
- if (rc)
- break;
+ if (rc) {
+ if (errno != ETIMEDOUT)
+ return rc;
+ c = NAK;
+ }
if (c != ACK && c != NAK && c != CAN)
printf("%c", c);
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 05/39] tools: kwboot: Print version information header
On 24.09.21 23:06, Marek Behún wrote:
From: Pali Rohár
Print kwboot's (U-Boot's) version when printing usage.
Signed-off-by: Pali Rohár
Reviewed-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 22cdd137ad..454339db14 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -11,6 +11,7 @@
#include "kwbimage.h"
#include "mkimage.h"
+#include "version.h"
#include
#include
@@ -681,6 +682,7 @@ out:
static void
kwboot_usage(FILE *stream, char *progname)
{
+ fprintf(stream, "kwboot version %s\n", PLAIN_VERSION);
fprintf(stream,
"Usage: %s [OPTIONS] [-b | -D ] [-B ]
\n",
progname);
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 04/39] tools: kwboot: Refactor and fix writing buffer
On 24.09.21 23:06, Marek Behún wrote:
From: Marek Behún
There are 3 instances in kwboot.c where we need to write() a given
buffer whole (iteratively writing until all data are written), and 2 of
those instances are wrong, for they do not increment the buffer pointer.
Refactor the code into a new function kwboot_write() where it is fixed.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 55 --
1 file changed, 26 insertions(+), 29 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index f18f6d2134..22cdd137ad 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -72,6 +72,23 @@ static int msg_req_delay = KWBOOT_MSG_REQ_DELAY;
static int msg_rsp_timeo = KWBOOT_MSG_RSP_TIMEO;
static int blk_rsp_timeo = KWBOOT_BLK_RSP_TIMEO;
+static ssize_t
+kwboot_write(int fd, const char *buf, size_t len)
+{
+ size_t tot = 0;
+
+ while (tot < len) {
+ ssize_t wr = write(fd, buf + tot, len - tot);
+
+ if (wr < 0)
+ return -1;
+
+ tot += wr;
+ }
+
+ return tot;
+}
+
static void
kwboot_printv(const char *fmt, ...)
{
@@ -191,26 +208,13 @@ out:
static int
kwboot_tty_send(int fd, const void *buf, size_t len)
{
- int rc;
- ssize_t n;
-
if (!buf)
return 0;
- rc = -1;
-
- do {
- n = write(fd, buf, len);
- if (n < 0)
- goto out;
-
- buf = (char *)buf + n;
- len -= n;
- } while (len > 0);
+ if (kwboot_write(fd, buf, len) < 0)
+ return -1;
- rc = tcdrain(fd);
-out:
- return rc;
+ return tcdrain(fd);
}
static int
@@ -462,7 +466,7 @@ can:
static int
kwboot_term_pipe(int in, int out, const char *quit, int *s)
{
- ssize_t nin, nout;
+ ssize_t nin;
char _buf[128], *buf = _buf;
nin = read(in, buf, sizeof(_buf));
@@ -480,22 +484,15 @@ kwboot_term_pipe(int in, int out, const char *quit, int
*s)
buf++;
nin--;
} else {
- while (*s > 0) {
- nout = write(out, quit, *s);
- if (nout <= 0)
- return -1;
- (*s) -= nout;
- }
+ if (kwboot_write(out, quit, *s) < 0)
+ return -1;
+ *s = 0;
}
}
}
- while (nin > 0) {
- nout = write(out, buf, nin);
- if (nout <= 0)
- return -1;
- nin -= nout;
- }
+ if (kwboot_write(out, buf, nin) < 0)
+ return -1;
return 0;
}
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 03/39] tools: kwboot: Make the quit sequence buffer const
On 24.09.21 23:06, Marek Behún wrote:
From: Marek Behún
This buffer is never written to. Make it const.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index e6e99849a7..f18f6d2134 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -460,7 +460,7 @@ can:
}
static int
-kwboot_term_pipe(int in, int out, char *quit, int *s)
+kwboot_term_pipe(int in, int out, const char *quit, int *s)
{
ssize_t nin, nout;
char _buf[128], *buf = _buf;
@@ -504,7 +504,7 @@ static int
kwboot_terminal(int tty)
{
int rc, in, s;
- char *quit = "\34c";
+ const char *quit = "\34c";
struct termios otio, tio;
rc = -1;
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 02/39] tools: kwboot: Fix buffer overflow in kwboot_terminal()
On 24.09.21 23:06, Marek Behún wrote:
From: Marek Behún
The `in` variable is set to -1 in kwboot_terminal() if stdin is not a
tty. In this case we should not look whether -1 is set in fd_set, for it
can lead to a buffer overflow, which can be reproduced with
echo "xyz" | ./tools/kwboot -t /dev/ttyUSB0
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwboot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 7feeaa45a2..e6e99849a7 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -552,7 +552,7 @@ kwboot_terminal(int tty)
break;
}
- if (FD_ISSET(in, &rfds)) {
+ if (in >= 0 && FD_ISSET(in, &rfds)) {
rc = kwboot_term_pipe(in, tty, quit, &s);
if (rc)
break;
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH u-boot-marvell v3 01/39] tools: kwbimage: Fix printf format warning
On 24.09.21 23:06, Marek Behún wrote:
From: Marek Behún
On 32-bit ARM the compiler complains:
tools/kwbimage.c:547: warning: format ‘%lu’ expects argument of type
‘long unsigned int’, but argument 4 has
type ‘unsigned int’
Fix this by using %zu instead of %lu format specifier.
Signed-off-by: Marek Behún
Reviewed-by: Stefan Roese
Thanks,
Stefan
---
tools/kwbimage.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index d200ff2425..e72555fe74 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -542,7 +542,7 @@ static int kwb_export_pubkey(RSA *key, struct pubkey_der_v1
*dst, FILE *hashf,
}
if (4 + size_seq > sizeof(dst->key)) {
- fprintf(stderr, "export pk failed: seq too large (%d, %lu)\n",
+ fprintf(stderr, "export pk failed: seq too large (%d, %zu)\n",
4 + size_seq, sizeof(dst->key));
fprintf(stderr, errmsg, keyname);
return -ENOBUFS;
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
[RFC 22/22] efi_selftest: block device: adjust dp for a test disk
Due to efi_disk-dm integration, the resultant device path for a test disk
got slightly changed, with efi_root contained as the first component.
Signed-off-by: AKASHI Takahiro
---
lib/efi_selftest/efi_selftest_block_device.c | 26 ++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/lib/efi_selftest/efi_selftest_block_device.c
b/lib/efi_selftest/efi_selftest_block_device.c
index 15f03751ac87..cac76249e6b4 100644
--- a/lib/efi_selftest/efi_selftest_block_device.c
+++ b/lib/efi_selftest/efi_selftest_block_device.c
@@ -30,6 +30,9 @@ static const efi_guid_t guid_device_path =
EFI_DEVICE_PATH_PROTOCOL_GUID;
static const efi_guid_t guid_simple_file_system_protocol =
EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID;
static const efi_guid_t guid_file_system_info = EFI_FILE_SYSTEM_INFO_GUID;
+static efi_guid_t guid_uboot =
+ EFI_GUID(0xe61d73b9, 0xa384, 0x4acc, \
+0xae, 0xab, 0x82, 0xe8, 0x28, 0xf3, 0x62, 0x8b);
static efi_guid_t guid_vendor =
EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d,
0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xb7, 0xb8);
@@ -206,25 +209,44 @@ static int setup(const efi_handle_t handle,
ret = boottime->allocate_pool(EFI_LOADER_DATA,
sizeof(struct efi_device_path_vendor) +
+ sizeof(struct efi_device_path_vendor) +
+ sizeof(u8) +
sizeof(struct efi_device_path),
(void **)&dp);
if (ret != EFI_SUCCESS) {
efi_st_error("Out of memory\n");
return EFI_ST_FAILURE;
}
+ /* first part */
vendor_node.dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
vendor_node.dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
vendor_node.dp.length = sizeof(struct efi_device_path_vendor);
- boottime->copy_mem(&vendor_node.guid, &guid_vendor,
+ boottime->copy_mem(&vendor_node.guid, &guid_uboot,
sizeof(efi_guid_t));
boottime->copy_mem(dp, &vendor_node,
sizeof(struct efi_device_path_vendor));
+
+ /* second part */
+ vendor_node.dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
+ vendor_node.dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
+ vendor_node.dp.length = sizeof(struct efi_device_path_vendor) + 1;
+
+ boottime->copy_mem(&vendor_node.guid, &guid_vendor,
+ sizeof(efi_guid_t));
+ boottime->copy_mem((char *)dp + sizeof(struct efi_device_path_vendor),
+ &vendor_node,
+ sizeof(struct efi_device_path_vendor));
+ /* vendor_data[0] */
+ *((char *)dp + sizeof(struct efi_device_path_vendor) * 2) = 0;
+
end_node.type = DEVICE_PATH_TYPE_END;
end_node.sub_type = DEVICE_PATH_SUB_TYPE_END;
end_node.length = sizeof(struct efi_device_path);
- boottime->copy_mem((char *)dp + sizeof(struct efi_device_path_vendor),
+ boottime->copy_mem((char *)dp + sizeof(struct efi_device_path_vendor)
+ + sizeof(struct efi_device_path_vendor)
+ + sizeof(u8),
&end_node, sizeof(struct efi_device_path));
ret = boottime->install_protocol_interface(&disk_handle,
&guid_device_path,
--
2.33.0
[RFC 22/22] (TEST) let dm-tree unchanged after block_io testing is done
---
lib/efi_selftest/efi_selftest_block_device.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_block_device.c
b/lib/efi_selftest/efi_selftest_block_device.c
index cac76249e6b4..358797d224dc 100644
--- a/lib/efi_selftest/efi_selftest_block_device.c
+++ b/lib/efi_selftest/efi_selftest_block_device.c
@@ -268,6 +268,7 @@ static int teardown(void)
{
efi_status_t r = EFI_ST_SUCCESS;
+#if 0 /* TEMP */
if (disk_handle) {
r = boottime->uninstall_protocol_interface(disk_handle,
&guid_device_path,
@@ -285,6 +286,7 @@ static int teardown(void)
return EFI_ST_FAILURE;
}
}
+#endif
if (image) {
r = boottime->free_pool(image);
--
2.33.0
[RFC 21/22] efi_selftest: block device: adjust dp for a test disk
Due to efi_disk-dm integration, the resultant device path for a test disk
got slightly changed, with efi_root contained as the first component.
Signed-off-by: AKASHI Takahiro
---
lib/efi_selftest/efi_selftest_block_device.c | 26 ++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/lib/efi_selftest/efi_selftest_block_device.c
b/lib/efi_selftest/efi_selftest_block_device.c
index 15f03751ac87..cac76249e6b4 100644
--- a/lib/efi_selftest/efi_selftest_block_device.c
+++ b/lib/efi_selftest/efi_selftest_block_device.c
@@ -30,6 +30,9 @@ static const efi_guid_t guid_device_path =
EFI_DEVICE_PATH_PROTOCOL_GUID;
static const efi_guid_t guid_simple_file_system_protocol =
EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID;
static const efi_guid_t guid_file_system_info = EFI_FILE_SYSTEM_INFO_GUID;
+static efi_guid_t guid_uboot =
+ EFI_GUID(0xe61d73b9, 0xa384, 0x4acc, \
+0xae, 0xab, 0x82, 0xe8, 0x28, 0xf3, 0x62, 0x8b);
static efi_guid_t guid_vendor =
EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d,
0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xb7, 0xb8);
@@ -206,25 +209,44 @@ static int setup(const efi_handle_t handle,
ret = boottime->allocate_pool(EFI_LOADER_DATA,
sizeof(struct efi_device_path_vendor) +
+ sizeof(struct efi_device_path_vendor) +
+ sizeof(u8) +
sizeof(struct efi_device_path),
(void **)&dp);
if (ret != EFI_SUCCESS) {
efi_st_error("Out of memory\n");
return EFI_ST_FAILURE;
}
+ /* first part */
vendor_node.dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
vendor_node.dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
vendor_node.dp.length = sizeof(struct efi_device_path_vendor);
- boottime->copy_mem(&vendor_node.guid, &guid_vendor,
+ boottime->copy_mem(&vendor_node.guid, &guid_uboot,
sizeof(efi_guid_t));
boottime->copy_mem(dp, &vendor_node,
sizeof(struct efi_device_path_vendor));
+
+ /* second part */
+ vendor_node.dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
+ vendor_node.dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
+ vendor_node.dp.length = sizeof(struct efi_device_path_vendor) + 1;
+
+ boottime->copy_mem(&vendor_node.guid, &guid_vendor,
+ sizeof(efi_guid_t));
+ boottime->copy_mem((char *)dp + sizeof(struct efi_device_path_vendor),
+ &vendor_node,
+ sizeof(struct efi_device_path_vendor));
+ /* vendor_data[0] */
+ *((char *)dp + sizeof(struct efi_device_path_vendor) * 2) = 0;
+
end_node.type = DEVICE_PATH_TYPE_END;
end_node.sub_type = DEVICE_PATH_SUB_TYPE_END;
end_node.length = sizeof(struct efi_device_path);
- boottime->copy_mem((char *)dp + sizeof(struct efi_device_path_vendor),
+ boottime->copy_mem((char *)dp + sizeof(struct efi_device_path_vendor)
+ + sizeof(struct efi_device_path_vendor)
+ + sizeof(u8),
&end_node, sizeof(struct efi_device_path));
ret = boottime->install_protocol_interface(&disk_handle,
&guid_device_path,
--
2.33.0
[RFC 21/22] efi_driver: cleanup after efi_disk-dm integration
efi_driver-specific binding will be no longer needed now that efi_disk-
dm integration takes care of efi_driver case as well.
Signed-off-by: AKASHI Takahiro
---
lib/efi_driver/efi_block_device.c | 24
1 file changed, 24 deletions(-)
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index b6afa939e1d1..1f39c93f7754 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -106,25 +106,6 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t
blknr, lbaint_t blkcnt,
return blkcnt;
}
-/**
- * Create partions for the block device.
- *
- * @handle:EFI handle of the block device
- * @dev: udevice of the block device
- * Return: number of partitions created
- */
-static int efi_bl_bind_partitions(efi_handle_t handle, struct udevice *dev)
-{
- struct blk_desc *desc;
- const char *if_typename;
-
- desc = dev_get_uclass_plat(dev);
- if_typename = blk_get_if_type_name(desc->if_type);
-
- return efi_disk_create_partitions(handle, desc, if_typename,
- desc->devnum, dev->name);
-}
-
/**
* Create a block device for a handle
*
@@ -139,7 +120,6 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
char *name;
struct efi_object *obj = efi_search_obj(handle);
struct efi_block_io *io = interface;
- int disks;
struct efi_blk_plat *plat;
EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, io);
@@ -184,10 +164,6 @@ static int efi_bl_bind(efi_handle_t handle, void
*interface)
return ret;
EFI_PRINT("%s: block device '%s' created\n", __func__, bdev->name);
- /* Create handles for the partions of the block device */
- disks = efi_bl_bind_partitions(handle, bdev);
- EFI_PRINT("Found %d partitions\n", disks);
-
return 0;
}
--
2.33.0
[RFC 20/22] efi_driver: cleanup after efi_disk-dm integration
efi_driver-specific binding will be no longer needed now that efi_disk-
dm integration takes care of efi_driver case as well.
Signed-off-by: AKASHI Takahiro
---
lib/efi_driver/efi_block_device.c | 24
1 file changed, 24 deletions(-)
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index b6afa939e1d1..1f39c93f7754 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -106,25 +106,6 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t
blknr, lbaint_t blkcnt,
return blkcnt;
}
-/**
- * Create partions for the block device.
- *
- * @handle:EFI handle of the block device
- * @dev: udevice of the block device
- * Return: number of partitions created
- */
-static int efi_bl_bind_partitions(efi_handle_t handle, struct udevice *dev)
-{
- struct blk_desc *desc;
- const char *if_typename;
-
- desc = dev_get_uclass_plat(dev);
- if_typename = blk_get_if_type_name(desc->if_type);
-
- return efi_disk_create_partitions(handle, desc, if_typename,
- desc->devnum, dev->name);
-}
-
/**
* Create a block device for a handle
*
@@ -139,7 +120,6 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
char *name;
struct efi_object *obj = efi_search_obj(handle);
struct efi_block_io *io = interface;
- int disks;
struct efi_blk_plat *plat;
EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, io);
@@ -184,10 +164,6 @@ static int efi_bl_bind(efi_handle_t handle, void
*interface)
return ret;
EFI_PRINT("%s: block device '%s' created\n", __func__, bdev->name);
- /* Create handles for the partions of the block device */
- disks = efi_bl_bind_partitions(handle, bdev);
- EFI_PRINT("Found %d partitions\n", disks);
-
return 0;
}
--
2.33.0
[RFC 20/22] efi_driver: align with efi_disk-dm integration
Signed-off-by: AKASHI Takahiro
---
lib/efi_driver/efi_block_device.c | 6 ++
lib/efi_loader/efi_device_path.c | 29 +
lib/efi_loader/efi_disk.c | 12 +++-
3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index 0937e3595a43..b6afa939e1d1 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -173,6 +173,12 @@ static int efi_bl_bind(efi_handle_t handle, void
*interface)
plat->handle = handle;
plat->io = interface;
+ /*
+* FIXME: necessary because we won't do almost nothing in
+* efi_disk_create() when called from device_probe().
+*/
+ bdev->efi_obj = handle;
+
ret = device_probe(bdev);
if (ret)
return ret;
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index cbdb466da41c..36c77bce9a05 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -628,6 +628,35 @@ __maybe_unused static void *dp_fill(void *buf, struct
udevice *dev)
return &dp->vendor_data[1];
}
#endif
+#ifdef CONFIG_EFI_LOADER
+ /*
+* FIXME: conflicting with CONFIG_SANDBOX
+* This case is necessary to support efi_disk's created by
+* efi_driver (and efi_driver_binding_protocol).
+* TODO:
+* The best way to work around here is to create efi_root as
+* udevice and put all efi_driver objects under it.
+*/
+ case UCLASS_ROOT: {
+ struct efi_device_path_vendor *dp;
+ struct blk_desc *desc = dev_get_uclass_plat(dev);
+ /* FIXME: guid_vendor used in selftest_block_device */
+ static efi_guid_t guid_vendor =
+ EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d,
+ 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xb7, 0xb8);
+
+
+ dp_fill(buf, dev->parent);
+ dp = buf;
+ ++dp;
+ dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
+ dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
+ dp->dp.length = sizeof(*dp) + 1;
+ memcpy(&dp->guid, &guid_vendor, sizeof(efi_guid_t));
+ dp->vendor_data[0] = desc->devnum;
+ return &dp->vendor_data[1];
+ }
+#endif
#ifdef CONFIG_VIRTIO_BLK
case UCLASS_VIRTIO: {
struct efi_device_path_vendor *dp;
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index dfd06dd31e4a..e7cf1567929b 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -562,11 +562,21 @@ static int efi_disk_create_part(struct udevice *dev)
int efi_disk_create(struct udevice *dev)
{
enum uclass_id id;
+ struct blk_desc *desc;
id = device_get_uclass_id(dev);
- if (id == UCLASS_BLK)
+ if (id == UCLASS_BLK) {
+ /*
+* avoid creating duplicated objects now that efi_driver
+* has already created an efi_disk at this moment.
+*/
+ desc = dev_get_uclass_plat(dev);
+ if (desc->if_type == IF_TYPE_EFI)
+ return 0;
+
return efi_disk_create_raw(dev);
+ }
if (id == UCLASS_PARTITION)
return efi_disk_create_part(dev);
--
2.33.0
[RFC 19/22] efi_driver: align with efi_disk-dm integration
Signed-off-by: AKASHI Takahiro
---
lib/efi_driver/efi_block_device.c | 6 ++
lib/efi_loader/efi_device_path.c | 29 +
lib/efi_loader/efi_disk.c | 12 +++-
3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/lib/efi_driver/efi_block_device.c
b/lib/efi_driver/efi_block_device.c
index 0937e3595a43..b6afa939e1d1 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -173,6 +173,12 @@ static int efi_bl_bind(efi_handle_t handle, void
*interface)
plat->handle = handle;
plat->io = interface;
+ /*
+* FIXME: necessary because we won't do almost nothing in
+* efi_disk_create() when called from device_probe().
+*/
+ bdev->efi_obj = handle;
+
ret = device_probe(bdev);
if (ret)
return ret;
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index cbdb466da41c..36c77bce9a05 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -628,6 +628,35 @@ __maybe_unused static void *dp_fill(void *buf, struct
udevice *dev)
return &dp->vendor_data[1];
}
#endif
+#ifdef CONFIG_EFI_LOADER
+ /*
+* FIXME: conflicting with CONFIG_SANDBOX
+* This case is necessary to support efi_disk's created by
+* efi_driver (and efi_driver_binding_protocol).
+* TODO:
+* The best way to work around here is to create efi_root as
+* udevice and put all efi_driver objects under it.
+*/
+ case UCLASS_ROOT: {
+ struct efi_device_path_vendor *dp;
+ struct blk_desc *desc = dev_get_uclass_plat(dev);
+ /* FIXME: guid_vendor used in selftest_block_device */
+ static efi_guid_t guid_vendor =
+ EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d,
+ 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xb7, 0xb8);
+
+
+ dp_fill(buf, dev->parent);
+ dp = buf;
+ ++dp;
+ dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
+ dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
+ dp->dp.length = sizeof(*dp) + 1;
+ memcpy(&dp->guid, &guid_vendor, sizeof(efi_guid_t));
+ dp->vendor_data[0] = desc->devnum;
+ return &dp->vendor_data[1];
+ }
+#endif
#ifdef CONFIG_VIRTIO_BLK
case UCLASS_VIRTIO: {
struct efi_device_path_vendor *dp;
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index dfd06dd31e4a..e7cf1567929b 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -562,11 +562,21 @@ static int efi_disk_create_part(struct udevice *dev)
int efi_disk_create(struct udevice *dev)
{
enum uclass_id id;
+ struct blk_desc *desc;
id = device_get_uclass_id(dev);
- if (id == UCLASS_BLK)
+ if (id == UCLASS_BLK) {
+ /*
+* avoid creating duplicated objects now that efi_driver
+* has already created an efi_disk at this moment.
+*/
+ desc = dev_get_uclass_plat(dev);
+ if (desc->if_type == IF_TYPE_EFI)
+ return 0;
+
return efi_disk_create_raw(dev);
+ }
if (id == UCLASS_PARTITION)
return efi_disk_create_part(dev);
--
2.33.0
[RFC 19/22] dm: blk: call efi's device-removal hook
Adding the callback function, efi_disk_delete(), in block devices's
pre_remove hook will allows for automatically deleting efi_disk objects
per block device.
This will eliminate any improper efi_disk objects which hold a link to
non-existing udevice structures when associated block devices are physically
un-plugged or udevices are once removed (and re-created) by executing commands
like "scsi rescan."
Signed-off-by: AKASHI Takahiro
---
drivers/block/blk-uclass.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index ce45cf0a8768..b8ad267c6c61 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -847,6 +847,16 @@ static int blk_post_probe(struct udevice *dev)
return 0;
}
+static int blk_pre_remove(struct udevice *dev)
+{
+ if (CONFIG_IS_ENABLED(EFI_LOADER)) {
+ if (efi_disk_delete(dev))
+ debug("*** efi_pre_remove_device failed\n");
+ }
+
+ return 0;
+}
+
static int blk_part_post_probe(struct udevice *dev)
{
if (CONFIG_IS_ENABLED(EFI_LOADER)) {
@@ -861,10 +871,21 @@ static int blk_part_post_probe(struct udevice *dev)
return 0;
}
+static int blk_part_pre_remove(struct udevice *dev)
+{
+ if (CONFIG_IS_ENABLED(EFI_LOADER)) {
+ if (efi_disk_delete(dev))
+ debug("*** efi_pre_remove_device failed\n");
+ }
+
+ return 0;
+}
+
UCLASS_DRIVER(blk) = {
.id = UCLASS_BLK,
.name = "blk",
.post_probe = blk_post_probe,
+ .pre_remove = blk_pre_remove,
.per_device_plat_auto = sizeof(struct blk_desc),
};
@@ -937,6 +958,7 @@ U_BOOT_DRIVER(blk_partition) = {
UCLASS_DRIVER(partition) = {
.id = UCLASS_PARTITION,
.post_probe = blk_part_post_probe,
+ .pre_remove = blk_part_pre_remove,
.per_device_plat_auto = sizeof(struct disk_part),
.name = "partition",
};
--
2.33.0
[RFC 18/22] efi_loader: efi_disk: a helper function to delete efi_disk objects
This function is expected to be called, in particular from dm's pre_remove
hook, when associated block devices no longer exist.
Signed-off-by: AKASHI Takahiro
---
include/efi_loader.h | 2 ++
lib/efi_loader/efi_disk.c | 54 +++
2 files changed, 56 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 50f4119dcdfb..7079549bea70 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -519,6 +519,8 @@ void efi_carve_out_dt_rsv(void *fdt);
efi_status_t efi_console_register(void);
/* Called when a block devices has been probed */
int efi_disk_create(struct udevice *dev);
+/* Called when a block devices is to be removed */
+int efi_disk_delete(struct udevice *dev);
/* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
efi_status_t efi_rng_register(void);
/* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 74ef923d1d67..dfd06dd31e4a 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -574,6 +574,60 @@ int efi_disk_create(struct udevice *dev)
return -1;
}
+static int efi_disk_delete_raw(struct udevice *dev)
+{
+ efi_handle_t handle = dev->efi_obj;
+ struct blk_desc *desc;
+ struct efi_disk_obj *diskobj;
+
+ desc = dev_get_uclass_plat(dev);
+ if (desc->if_type != IF_TYPE_EFI) {
+ diskobj = container_of(handle, struct efi_disk_obj, header);
+ efi_free_pool(diskobj->dp);
+ }
+
+ /*
+* TODO: Can we use efi_delete_handle() here?
+*/
+ efi_remove_all_protocols(handle);
+
+ efi_remove_handle(handle);
+ free(diskobj);
+
+ return 0;
+}
+
+static int efi_disk_delete_part(struct udevice *dev)
+{
+ efi_handle_t handle = dev->efi_obj;
+ struct efi_disk_obj *diskobj;
+
+ diskobj = container_of(handle, struct efi_disk_obj, header);
+
+ efi_free_pool(diskobj->dp);
+
+ efi_remove_all_protocols(handle);
+
+ efi_remove_handle(handle);
+ free(diskobj);
+
+ return 0;
+}
+
+int efi_disk_delete(struct udevice *dev)
+{
+ enum uclass_id id;
+
+ id = device_get_uclass_id(dev);
+
+ if (id == UCLASS_BLK)
+ return efi_disk_delete_raw(dev);
+ else if (id == UCLASS_PARTITION)
+ return efi_disk_delete_part(dev);
+ else
+ return -1;
+}
+
/**
* efi_disk_is_system_part() - check if handle refers to an EFI system
partition
*
--
2.33.0
[RFC 18/22] dm: blk: call efi's device-removal hook
Adding the callback function, efi_disk_delete(), in block devices's
pre_remove hook will allows for automatically deleting efi_disk objects
per block device.
This will eliminate any improper efi_disk objects which hold a link to
non-existing udevice structures when associated block devices are physically
un-plugged or udevices are once removed (and re-created) by executing commands
like "scsi rescan."
Signed-off-by: AKASHI Takahiro
---
drivers/block/blk-uclass.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index ce45cf0a8768..b8ad267c6c61 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -847,6 +847,16 @@ static int blk_post_probe(struct udevice *dev)
return 0;
}
+static int blk_pre_remove(struct udevice *dev)
+{
+ if (CONFIG_IS_ENABLED(EFI_LOADER)) {
+ if (efi_disk_delete(dev))
+ debug("*** efi_pre_remove_device failed\n");
+ }
+
+ return 0;
+}
+
static int blk_part_post_probe(struct udevice *dev)
{
if (CONFIG_IS_ENABLED(EFI_LOADER)) {
@@ -861,10 +871,21 @@ static int blk_part_post_probe(struct udevice *dev)
return 0;
}
+static int blk_part_pre_remove(struct udevice *dev)
+{
+ if (CONFIG_IS_ENABLED(EFI_LOADER)) {
+ if (efi_disk_delete(dev))
+ debug("*** efi_pre_remove_device failed\n");
+ }
+
+ return 0;
+}
+
UCLASS_DRIVER(blk) = {
.id = UCLASS_BLK,
.name = "blk",
.post_probe = blk_post_probe,
+ .pre_remove = blk_pre_remove,
.per_device_plat_auto = sizeof(struct blk_desc),
};
@@ -937,6 +958,7 @@ U_BOOT_DRIVER(blk_partition) = {
UCLASS_DRIVER(partition) = {
.id = UCLASS_PARTITION,
.post_probe = blk_part_post_probe,
+ .pre_remove = blk_part_pre_remove,
.per_device_plat_auto = sizeof(struct disk_part),
.name = "partition",
};
--
2.33.0
[RFC 17/22] efi_loader: efi_disk: a helper function to delete efi_disk objects
This function is expected to be called, in particular from dm's pre_remove
hook, when associated block devices no longer exist.
Signed-off-by: AKASHI Takahiro
---
include/efi_loader.h | 2 ++
lib/efi_loader/efi_disk.c | 54 +++
2 files changed, 56 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 50f4119dcdfb..7079549bea70 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -519,6 +519,8 @@ void efi_carve_out_dt_rsv(void *fdt);
efi_status_t efi_console_register(void);
/* Called when a block devices has been probed */
int efi_disk_create(struct udevice *dev);
+/* Called when a block devices is to be removed */
+int efi_disk_delete(struct udevice *dev);
/* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
efi_status_t efi_rng_register(void);
/* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 74ef923d1d67..dfd06dd31e4a 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -574,6 +574,60 @@ int efi_disk_create(struct udevice *dev)
return -1;
}
+static int efi_disk_delete_raw(struct udevice *dev)
+{
+ efi_handle_t handle = dev->efi_obj;
+ struct blk_desc *desc;
+ struct efi_disk_obj *diskobj;
+
+ desc = dev_get_uclass_plat(dev);
+ if (desc->if_type != IF_TYPE_EFI) {
+ diskobj = container_of(handle, struct efi_disk_obj, header);
+ efi_free_pool(diskobj->dp);
+ }
+
+ /*
+* TODO: Can we use efi_delete_handle() here?
+*/
+ efi_remove_all_protocols(handle);
+
+ efi_remove_handle(handle);
+ free(diskobj);
+
+ return 0;
+}
+
+static int efi_disk_delete_part(struct udevice *dev)
+{
+ efi_handle_t handle = dev->efi_obj;
+ struct efi_disk_obj *diskobj;
+
+ diskobj = container_of(handle, struct efi_disk_obj, header);
+
+ efi_free_pool(diskobj->dp);
+
+ efi_remove_all_protocols(handle);
+
+ efi_remove_handle(handle);
+ free(diskobj);
+
+ return 0;
+}
+
+int efi_disk_delete(struct udevice *dev)
+{
+ enum uclass_id id;
+
+ id = device_get_uclass_id(dev);
+
+ if (id == UCLASS_BLK)
+ return efi_disk_delete_raw(dev);
+ else if (id == UCLASS_PARTITION)
+ return efi_disk_delete_part(dev);
+ else
+ return -1;
+}
+
/**
* efi_disk_is_system_part() - check if handle refers to an EFI system
partition
*
--
2.33.0
[RFC 17/22] efi_loader: add efi_remove_handle()
This function is a counterpart of efi_add_handle() and will be used
in order to remove an efi_disk object in a later patch.
Signed-off-by: AKASHI Takahiro
---
include/efi_loader.h | 2 ++
lib/efi_loader/efi_boottime.c | 8
2 files changed, 10 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index cfbe1fe659ef..50f4119dcdfb 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -579,6 +579,8 @@ void efi_save_gd(void);
void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map);
/* Add a new object to the object list. */
void efi_add_handle(efi_handle_t obj);
+/* Remove a object from the object list. */
+void efi_remove_handle(efi_handle_t obj);
/* Create handle */
efi_status_t efi_create_handle(efi_handle_t *handle);
/* Delete handle */
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index f0283b539e46..b2503b74233b 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -503,6 +503,14 @@ void efi_add_handle(efi_handle_t handle)
list_add_tail(&handle->link, &efi_obj_list);
}
+void efi_remove_handle(efi_handle_t handle)
+{
+ if (!handle)
+ return;
+
+ list_del(&handle->link);
+}
+
/**
* efi_create_handle() - create handle
* @handle: new handle
--
2.33.0
[RFC 16/22] efi_loader: cleanup after efi_disk-dm integration
efi_disk_register() will be no longer needed now that all efi_disks are
set to be created with device model thanks to efi_disk-dm integration.
Signed-off-by: AKASHI Takahiro
---
include/efi_loader.h | 2 -
lib/efi_loader/efi_disk.c | 102 -
lib/efi_loader/efi_setup.c | 5 --
3 files changed, 109 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 751fde7fb153..cfbe1fe659ef 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -519,8 +519,6 @@ void efi_carve_out_dt_rsv(void *fdt);
efi_status_t efi_console_register(void);
/* Called when a block devices has been probed */
int efi_disk_create(struct udevice *dev);
-/* Called by bootefi to make all disk storage accessible as EFI objects */
-efi_status_t efi_disk_register(void);
/* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
efi_status_t efi_rng_register(void);
/* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 3fae40e034fb..74ef923d1d67 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -485,56 +485,6 @@ error:
return ret;
}
-#ifndef CONFIG_BLK
-/**
- * efi_disk_create_partitions() - create handles and protocols for partitions
- *
- * Create handles and protocols for the partitions of a block device.
- *
- * @parent:handle of the parent disk
- * @desc: block device
- * @if_typename: interface type
- * @diskid:device number
- * @pdevname: device name
- * Return: number of partitions created
- */
-int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
- const char *if_typename, int diskid,
- const char *pdevname)
-{
- int disks = 0;
- char devname[32] = { 0 }; /* dp->str is u16[32] long */
- int part;
- struct efi_device_path *dp = NULL;
- efi_status_t ret;
- struct efi_handler *handler;
-
- /* Get the device path of the parent */
- ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
- if (ret == EFI_SUCCESS)
- dp = handler->protocol_interface;
-
- /* Add devices for each partition */
- for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
- struct disk_partition info;
-
- if (part_get_info(desc, part, &info))
- continue;
- snprintf(devname, sizeof(devname), "%s:%x", pdevname,
-part);
- ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
- &info, part, NULL);
- if (ret != EFI_SUCCESS) {
- log_err("Adding partition %s failed\n", pdevname);
- continue;
- }
- disks++;
- }
-
- return disks;
-}
-#endif /* CONFIG_BLK */
-
/*
* Create a handle for a whole raw disk
*
@@ -624,58 +574,6 @@ int efi_disk_create(struct udevice *dev)
return -1;
}
-/**
- * efi_disk_register() - register block devices
- *
- * U-Boot doesn't have a list of all online disk devices. So when running our
- * EFI payload, we scan through all of the potentially available ones and
- * store them in our object pool.
- *
- * This function is called in efi_init_obj_list().
- *
- * TODO(s...@chromium.org): Actually with CONFIG_BLK, U-Boot does have this.
- * Consider converting the code to look up devices as needed. The EFI device
- * could be a child of the UCLASS_BLK block device, perhaps.
- *
- * Return: status code
- */
-efi_status_t efi_disk_register(void)
-{
- struct efi_disk_obj *disk;
- int disks = 0;
- efi_status_t ret;
- struct udevice *dev;
-
- for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
-uclass_next_device_check(&dev)) {
- struct blk_desc *desc = dev_get_uclass_plat(dev);
- const char *if_typename = blk_get_if_type_name(desc->if_type);
-
- /* Add block device for the full device */
- log_info("Scanning disk %s...\n", dev->name);
- ret = efi_disk_add_dev(NULL, NULL, if_typename,
- desc, desc->devnum, NULL, 0, &disk);
- if (ret == EFI_NOT_READY) {
- log_notice("Disk %s not ready\n", dev->name);
- continue;
- }
- if (ret) {
- log_err("ERROR: failure to add disk device %s, r =
%lu\n",
- dev->name, ret & ~EFI_ERROR_MASK);
- return ret;
- }
- disks++;
-
- /* Partitions show up as block devices in EFI */
- disks += efi_disk_create_partitions(
- &disk->header, desc, if_type
[RFC 16/22] efi_loader: add efi_remove_handle()
This function is a counterpart of efi_add_handle() and will be used
in order to remove an efi_disk object in a later patch.
Signed-off-by: AKASHI Takahiro
---
include/efi_loader.h | 2 ++
lib/efi_loader/efi_boottime.c | 8
2 files changed, 10 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index cfbe1fe659ef..50f4119dcdfb 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -579,6 +579,8 @@ void efi_save_gd(void);
void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map);
/* Add a new object to the object list. */
void efi_add_handle(efi_handle_t obj);
+/* Remove a object from the object list. */
+void efi_remove_handle(efi_handle_t obj);
/* Create handle */
efi_status_t efi_create_handle(efi_handle_t *handle);
/* Delete handle */
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index f0283b539e46..b2503b74233b 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -503,6 +503,14 @@ void efi_add_handle(efi_handle_t handle)
list_add_tail(&handle->link, &efi_obj_list);
}
+void efi_remove_handle(efi_handle_t handle)
+{
+ if (!handle)
+ return;
+
+ list_del(&handle->link);
+}
+
/**
* efi_create_handle() - create handle
* @handle: new handle
--
2.33.0
[RFC 15/22] efi_loader: cleanup after efi_disk-dm integration
efi_disk_register() will be no longer needed now that all efi_disks are
set to be created with device model thanks to efi_disk-dm integration.
Signed-off-by: AKASHI Takahiro
---
include/efi_loader.h | 2 -
lib/efi_loader/efi_disk.c | 102 -
lib/efi_loader/efi_setup.c | 5 --
3 files changed, 109 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 751fde7fb153..cfbe1fe659ef 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -519,8 +519,6 @@ void efi_carve_out_dt_rsv(void *fdt);
efi_status_t efi_console_register(void);
/* Called when a block devices has been probed */
int efi_disk_create(struct udevice *dev);
-/* Called by bootefi to make all disk storage accessible as EFI objects */
-efi_status_t efi_disk_register(void);
/* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
efi_status_t efi_rng_register(void);
/* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 3fae40e034fb..74ef923d1d67 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -485,56 +485,6 @@ error:
return ret;
}
-#ifndef CONFIG_BLK
-/**
- * efi_disk_create_partitions() - create handles and protocols for partitions
- *
- * Create handles and protocols for the partitions of a block device.
- *
- * @parent:handle of the parent disk
- * @desc: block device
- * @if_typename: interface type
- * @diskid:device number
- * @pdevname: device name
- * Return: number of partitions created
- */
-int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
- const char *if_typename, int diskid,
- const char *pdevname)
-{
- int disks = 0;
- char devname[32] = { 0 }; /* dp->str is u16[32] long */
- int part;
- struct efi_device_path *dp = NULL;
- efi_status_t ret;
- struct efi_handler *handler;
-
- /* Get the device path of the parent */
- ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
- if (ret == EFI_SUCCESS)
- dp = handler->protocol_interface;
-
- /* Add devices for each partition */
- for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
- struct disk_partition info;
-
- if (part_get_info(desc, part, &info))
- continue;
- snprintf(devname, sizeof(devname), "%s:%x", pdevname,
-part);
- ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
- &info, part, NULL);
- if (ret != EFI_SUCCESS) {
- log_err("Adding partition %s failed\n", pdevname);
- continue;
- }
- disks++;
- }
-
- return disks;
-}
-#endif /* CONFIG_BLK */
-
/*
* Create a handle for a whole raw disk
*
@@ -624,58 +574,6 @@ int efi_disk_create(struct udevice *dev)
return -1;
}
-/**
- * efi_disk_register() - register block devices
- *
- * U-Boot doesn't have a list of all online disk devices. So when running our
- * EFI payload, we scan through all of the potentially available ones and
- * store them in our object pool.
- *
- * This function is called in efi_init_obj_list().
- *
- * TODO(s...@chromium.org): Actually with CONFIG_BLK, U-Boot does have this.
- * Consider converting the code to look up devices as needed. The EFI device
- * could be a child of the UCLASS_BLK block device, perhaps.
- *
- * Return: status code
- */
-efi_status_t efi_disk_register(void)
-{
- struct efi_disk_obj *disk;
- int disks = 0;
- efi_status_t ret;
- struct udevice *dev;
-
- for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
-uclass_next_device_check(&dev)) {
- struct blk_desc *desc = dev_get_uclass_plat(dev);
- const char *if_typename = blk_get_if_type_name(desc->if_type);
-
- /* Add block device for the full device */
- log_info("Scanning disk %s...\n", dev->name);
- ret = efi_disk_add_dev(NULL, NULL, if_typename,
- desc, desc->devnum, NULL, 0, &disk);
- if (ret == EFI_NOT_READY) {
- log_notice("Disk %s not ready\n", dev->name);
- continue;
- }
- if (ret) {
- log_err("ERROR: failure to add disk device %s, r =
%lu\n",
- dev->name, ret & ~EFI_ERROR_MASK);
- return ret;
- }
- disks++;
-
- /* Partitions show up as block devices in EFI */
- disks += efi_disk_create_partitions(
- &disk->header, desc, if_type
[RFC 15/22] dm: blk: call efi's device-probe hook
Adding this callback function, efi_disk_create() in block devices's
post_probe hook will allows for automatically creating efi_disk objects
per block device.
This will end up not only eliminating efi_disk_register() called in UEFI
initialization, but also enabling detections of new block devices even
after the initialization.
Signed-off-by: AKASHI Takahiro
---
drivers/block/blk-uclass.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 8fbec8779e1e..ce45cf0a8768 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -827,6 +828,11 @@ int blk_create_partitions(struct udevice *parent)
static int blk_post_probe(struct udevice *dev)
{
+ if (CONFIG_IS_ENABLED(EFI_LOADER)) {
+ if (efi_disk_create(dev))
+ debug("*** efi_post_probe_device failed\n");
+ }
+
if (IS_ENABLED(CONFIG_PARTITIONS) &&
IS_ENABLED(CONFIG_HAVE_BLOCK_DEVICE)) {
struct blk_desc *desc = dev_get_uclass_plat(dev);
@@ -843,6 +849,10 @@ static int blk_post_probe(struct udevice *dev)
static int blk_part_post_probe(struct udevice *dev)
{
+ if (CONFIG_IS_ENABLED(EFI_LOADER)) {
+ if (efi_disk_create(dev))
+ debug("*** efi_post_probe_device failed\n");
+ }
/*
* TODO:
* If we call blk_creat_partitions() here, it would allow for
--
2.33.0
[RFC 14/22] efi_loader: disk: a helper function to create efi_disk objects from udevice
Add efi_disk_create() function.
Any UEFI handle created by efi_disk_create() can be treated as a efi_disk
object, the udevice is either a UCLASS_BLK (a whole raw disk) or
UCLASS_PARTITION (a disk partition).
So this function is expected to be called every time such an udevice
is detected and activated through a device model's "probe" interface.
Signed-off-by: AKASHI Takahiro
---
include/efi_loader.h | 2 +
lib/efi_loader/efi_disk.c | 92 +++
2 files changed, 94 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index c440962fe522..751fde7fb153 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -517,6 +517,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t
debug_disposition,
void efi_carve_out_dt_rsv(void *fdt);
/* Called by bootefi to make console interface available */
efi_status_t efi_console_register(void);
+/* Called when a block devices has been probed */
+int efi_disk_create(struct udevice *dev);
/* Called by bootefi to make all disk storage accessible as EFI objects */
efi_status_t efi_disk_register(void);
/* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index cd5528046251..3fae40e034fb 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -484,6 +485,7 @@ error:
return ret;
}
+#ifndef CONFIG_BLK
/**
* efi_disk_create_partitions() - create handles and protocols for partitions
*
@@ -531,6 +533,96 @@ int efi_disk_create_partitions(efi_handle_t parent, struct
blk_desc *desc,
return disks;
}
+#endif /* CONFIG_BLK */
+
+/*
+ * Create a handle for a whole raw disk
+ *
+ * @devuclass device
+ * @return 0 on success, -1 otherwise
+ */
+static int efi_disk_create_raw(struct udevice *dev)
+{
+ struct efi_disk_obj *disk;
+ struct blk_desc *desc;
+ const char *if_typename;
+ int diskid;
+ efi_status_t ret;
+
+ desc = dev_get_uclass_plat(dev);
+ if_typename = blk_get_if_type_name(desc->if_type);
+ diskid = desc->devnum;
+
+ ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,
+ diskid, NULL, 0, &disk);
+ if (ret != EFI_SUCCESS) {
+ log_err("Adding disk %s%d failed\n", if_typename, diskid);
+ return -1;
+ }
+ disk->dev = dev;
+ dev->efi_obj = &disk->header;
+
+ return 0;
+}
+
+/*
+ * Create a handle for a disk partition
+ *
+ * @devuclass device
+ * @return 0 on success, -1 otherwise
+ */
+static int efi_disk_create_part(struct udevice *dev)
+{
+ efi_handle_t parent;
+ struct blk_desc *desc;
+ const char *if_typename;
+ struct disk_part *part_data;
+ struct disk_partition *info;
+ unsigned int part;
+ int diskid;
+ struct efi_device_path *dp = NULL;
+ struct efi_disk_obj *disk;
+ efi_status_t ret;
+
+ parent = dev->parent->efi_obj;
+ desc = dev_get_uclass_plat(dev->parent);
+ if_typename = blk_get_if_type_name(desc->if_type);
+ diskid = desc->devnum;
+
+ part_data = dev_get_uclass_plat(dev);
+ part = part_data->partnum;
+ info = &part_data->gpt_part_info;
+
+ /* TODO: should not use desc? */
+ dp = efi_dp_from_part(desc, 0);
+
+ ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
+ info, part, &disk);
+ if (ret != EFI_SUCCESS) {
+ log_err("Adding partition %s%d:%x failed\n",
+ if_typename, diskid, part);
+ return -1;
+ }
+ disk->dev = dev;
+ dev->efi_obj = &disk->header;
+
+ return 0;
+}
+
+int efi_disk_create(struct udevice *dev)
+{
+ enum uclass_id id;
+
+ id = device_get_uclass_id(dev);
+
+ if (id == UCLASS_BLK)
+ return efi_disk_create_raw(dev);
+
+ if (id == UCLASS_PARTITION)
+ return efi_disk_create_part(dev);
+
+ return -1;
+}
/**
* efi_disk_register() - register block devices
--
2.33.0
[RFC 14/22] dm: blk: call efi's device-probe hook
Adding this callback function, efi_disk_create() in block devices's
post_probe hook will allows for automatically creating efi_disk objects
per block device.
This will end up not only eliminating efi_disk_register() called in UEFI
initialization, but also enabling detections of new block devices even
after the initialization.
Signed-off-by: AKASHI Takahiro
---
drivers/block/blk-uclass.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 8fbec8779e1e..ce45cf0a8768 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -827,6 +828,11 @@ int blk_create_partitions(struct udevice *parent)
static int blk_post_probe(struct udevice *dev)
{
+ if (CONFIG_IS_ENABLED(EFI_LOADER)) {
+ if (efi_disk_create(dev))
+ debug("*** efi_post_probe_device failed\n");
+ }
+
if (IS_ENABLED(CONFIG_PARTITIONS) &&
IS_ENABLED(CONFIG_HAVE_BLOCK_DEVICE)) {
struct blk_desc *desc = dev_get_uclass_plat(dev);
@@ -843,6 +849,10 @@ static int blk_post_probe(struct udevice *dev)
static int blk_part_post_probe(struct udevice *dev)
{
+ if (CONFIG_IS_ENABLED(EFI_LOADER)) {
+ if (efi_disk_create(dev))
+ debug("*** efi_post_probe_device failed\n");
+ }
/*
* TODO:
* If we call blk_creat_partitions() here, it would allow for
--
2.33.0
[RFC 13/22] efi_loader: remove !CONFIG_BLK code from efi_disk
The change in this patch will probably have been covered by other guy's patch.
Signed-off-by: AKASHI Takahiro
---
lib/efi_loader/efi_disk.c | 49 ---
1 file changed, 49 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index dfa6f898d586..cd5528046251 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -552,7 +552,6 @@ efi_status_t efi_disk_register(void)
struct efi_disk_obj *disk;
int disks = 0;
efi_status_t ret;
-#ifdef CONFIG_BLK
struct udevice *dev;
for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
@@ -580,54 +579,6 @@ efi_status_t efi_disk_register(void)
&disk->header, desc, if_typename,
desc->devnum, dev->name);
}
-#else
- int i, if_type;
-
- /* Search for all available disk devices */
- for (if_type = 0; if_type < IF_TYPE_COUNT; if_type++) {
- const struct blk_driver *cur_drvr;
- const char *if_typename;
-
- cur_drvr = blk_driver_lookup_type(if_type);
- if (!cur_drvr)
- continue;
-
- if_typename = cur_drvr->if_typename;
- log_info("Scanning disks on %s...\n", if_typename);
- for (i = 0; i < 4; i++) {
- struct blk_desc *desc;
- char devname[32] = { 0 }; /* dp->str is u16[32] long */
-
- desc = blk_get_devnum_by_type(if_type, i);
- if (!desc)
- continue;
- if (desc->type == DEV_TYPE_UNKNOWN)
- continue;
-
- snprintf(devname, sizeof(devname), "%s%d",
-if_typename, i);
-
- /* Add block device for the full device */
- ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,
- i, NULL, 0, &disk);
- if (ret == EFI_NOT_READY) {
- log_notice("Disk %s not ready\n", devname);
- continue;
- }
- if (ret) {
- log_err("ERROR: failure to add disk device %s,
r = %lu\n",
- devname, ret & ~EFI_ERROR_MASK);
- return ret;
- }
- disks++;
-
- /* Partitions show up as block devices in EFI */
- disks += efi_disk_create_partitions
- (&disk->header, desc,
-if_typename, i, devname);
- }
- }
-#endif
log_info("Found %d disks\n", disks);
return EFI_SUCCESS;
--
2.33.0
[RFC 13/22] efi_loader: disk: a helper function to create efi_disk objects from udevice
Add efi_disk_create() function.
Any UEFI handle created by efi_disk_create() can be treated as a efi_disk
object, the udevice is either a UCLASS_BLK (a whole raw disk) or
UCLASS_PARTITION (a disk partition).
So this function is expected to be called every time such an udevice
is detected and activated through a device model's "probe" interface.
Signed-off-by: AKASHI Takahiro
---
include/efi_loader.h | 2 +
lib/efi_loader/efi_disk.c | 92 +++
2 files changed, 94 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index c440962fe522..751fde7fb153 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -517,6 +517,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t
debug_disposition,
void efi_carve_out_dt_rsv(void *fdt);
/* Called by bootefi to make console interface available */
efi_status_t efi_console_register(void);
+/* Called when a block devices has been probed */
+int efi_disk_create(struct udevice *dev);
/* Called by bootefi to make all disk storage accessible as EFI objects */
efi_status_t efi_disk_register(void);
/* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index cd5528046251..3fae40e034fb 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -484,6 +485,7 @@ error:
return ret;
}
+#ifndef CONFIG_BLK
/**
* efi_disk_create_partitions() - create handles and protocols for partitions
*
@@ -531,6 +533,96 @@ int efi_disk_create_partitions(efi_handle_t parent, struct
blk_desc *desc,
return disks;
}
+#endif /* CONFIG_BLK */
+
+/*
+ * Create a handle for a whole raw disk
+ *
+ * @devuclass device
+ * @return 0 on success, -1 otherwise
+ */
+static int efi_disk_create_raw(struct udevice *dev)
+{
+ struct efi_disk_obj *disk;
+ struct blk_desc *desc;
+ const char *if_typename;
+ int diskid;
+ efi_status_t ret;
+
+ desc = dev_get_uclass_plat(dev);
+ if_typename = blk_get_if_type_name(desc->if_type);
+ diskid = desc->devnum;
+
+ ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,
+ diskid, NULL, 0, &disk);
+ if (ret != EFI_SUCCESS) {
+ log_err("Adding disk %s%d failed\n", if_typename, diskid);
+ return -1;
+ }
+ disk->dev = dev;
+ dev->efi_obj = &disk->header;
+
+ return 0;
+}
+
+/*
+ * Create a handle for a disk partition
+ *
+ * @devuclass device
+ * @return 0 on success, -1 otherwise
+ */
+static int efi_disk_create_part(struct udevice *dev)
+{
+ efi_handle_t parent;
+ struct blk_desc *desc;
+ const char *if_typename;
+ struct disk_part *part_data;
+ struct disk_partition *info;
+ unsigned int part;
+ int diskid;
+ struct efi_device_path *dp = NULL;
+ struct efi_disk_obj *disk;
+ efi_status_t ret;
+
+ parent = dev->parent->efi_obj;
+ desc = dev_get_uclass_plat(dev->parent);
+ if_typename = blk_get_if_type_name(desc->if_type);
+ diskid = desc->devnum;
+
+ part_data = dev_get_uclass_plat(dev);
+ part = part_data->partnum;
+ info = &part_data->gpt_part_info;
+
+ /* TODO: should not use desc? */
+ dp = efi_dp_from_part(desc, 0);
+
+ ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
+ info, part, &disk);
+ if (ret != EFI_SUCCESS) {
+ log_err("Adding partition %s%d:%x failed\n",
+ if_typename, diskid, part);
+ return -1;
+ }
+ disk->dev = dev;
+ dev->efi_obj = &disk->header;
+
+ return 0;
+}
+
+int efi_disk_create(struct udevice *dev)
+{
+ enum uclass_id id;
+
+ id = device_get_uclass_id(dev);
+
+ if (id == UCLASS_BLK)
+ return efi_disk_create_raw(dev);
+
+ if (id == UCLASS_PARTITION)
+ return efi_disk_create_part(dev);
+
+ return -1;
+}
/**
* efi_disk_register() - register block devices
--
2.33.0
[RFC 12/22] efi_loader: remove !CONFIG_BLK code from efi_disk
The change in this patch will probably have been covered by other guy's patch.
Signed-off-by: AKASHI Takahiro
---
lib/efi_loader/efi_disk.c | 49 ---
1 file changed, 49 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index dfa6f898d586..cd5528046251 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -552,7 +552,6 @@ efi_status_t efi_disk_register(void)
struct efi_disk_obj *disk;
int disks = 0;
efi_status_t ret;
-#ifdef CONFIG_BLK
struct udevice *dev;
for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
@@ -580,54 +579,6 @@ efi_status_t efi_disk_register(void)
&disk->header, desc, if_typename,
desc->devnum, dev->name);
}
-#else
- int i, if_type;
-
- /* Search for all available disk devices */
- for (if_type = 0; if_type < IF_TYPE_COUNT; if_type++) {
- const struct blk_driver *cur_drvr;
- const char *if_typename;
-
- cur_drvr = blk_driver_lookup_type(if_type);
- if (!cur_drvr)
- continue;
-
- if_typename = cur_drvr->if_typename;
- log_info("Scanning disks on %s...\n", if_typename);
- for (i = 0; i < 4; i++) {
- struct blk_desc *desc;
- char devname[32] = { 0 }; /* dp->str is u16[32] long */
-
- desc = blk_get_devnum_by_type(if_type, i);
- if (!desc)
- continue;
- if (desc->type == DEV_TYPE_UNKNOWN)
- continue;
-
- snprintf(devname, sizeof(devname), "%s%d",
-if_typename, i);
-
- /* Add block device for the full device */
- ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,
- i, NULL, 0, &disk);
- if (ret == EFI_NOT_READY) {
- log_notice("Disk %s not ready\n", devname);
- continue;
- }
- if (ret) {
- log_err("ERROR: failure to add disk device %s,
r = %lu\n",
- devname, ret & ~EFI_ERROR_MASK);
- return ret;
- }
- disks++;
-
- /* Partitions show up as block devices in EFI */
- disks += efi_disk_create_partitions
- (&disk->header, desc,
-if_typename, i, devname);
- }
- }
-#endif
log_info("Found %d disks\n", disks);
return EFI_SUCCESS;
--
2.33.0
[RFC 12/22] dm: add a hidden link to efi object
This member field in udevice will be used to dereference from udevice
to efi_object (or efi_handle).
Signed-off-by: AKASHI Takahiro
---
include/dm/device.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/dm/device.h b/include/dm/device.h
index 0a9718a5b81a..33b09a836f06 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -190,6 +190,10 @@ struct udevice {
#if CONFIG_IS_ENABLED(DM_DMA)
ulong dma_offset;
#endif
+#if CONFIG_IS_ENABLED(EFI_LOADER)
+ /* link to efi_object */
+ void *efi_obj;
+#endif
};
/**
--
2.33.0
[RFC 11/22] efi_loader: disk: use udevice instead of blk_desc
In most of all usages, we can avoid accessing blk_desc which is eventually
an internal data structure to be hided outside block device drivers.
Signed-off-by: AKASHI Takahiro
---
lib/efi_loader/efi_disk.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 988907ecb910..dfa6f898d586 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -45,7 +45,7 @@ struct efi_disk_obj {
unsigned int part;
struct efi_simple_file_system_protocol *volume;
lbaint_t offset;
- struct blk_desc *desc;
+ struct udevice *dev; /* TODO: move it to efi_object */
};
/**
@@ -80,14 +80,12 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io
*this,
void *buffer, enum efi_disk_direction direction)
{
struct efi_disk_obj *diskobj;
- struct blk_desc *desc;
int blksz;
int blocks;
unsigned long n;
diskobj = container_of(this, struct efi_disk_obj, ops);
- desc = (struct blk_desc *) diskobj->desc;
- blksz = desc->blksz;
+ blksz = diskobj->media.block_size;
blocks = buffer_size / blksz;
lba += diskobj->offset;
@@ -99,9 +97,9 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io
*this,
return EFI_BAD_BUFFER_SIZE;
if (direction == EFI_DISK_READ)
- n = blk_dread(desc, lba, blocks, buffer);
+ n = blk_read(diskobj->dev, lba, blocks, buffer);
else
- n = blk_dwrite(desc, lba, blocks, buffer);
+ n = blk_write(diskobj->dev, lba, blocks, buffer);
/* We don't do interrupts, so check for timers cooperatively */
efi_timer_check();
@@ -443,7 +441,6 @@ static efi_status_t efi_disk_add_dev(
diskobj->ops = block_io_disk_template;
diskobj->ifname = if_typename;
diskobj->dev_index = dev_index;
- diskobj->desc = desc;
/* Fill in EFI IO Media info (for read/write callbacks) */
diskobj->media.removable_media = desc->removable;
@@ -647,20 +644,22 @@ bool efi_disk_is_system_part(efi_handle_t handle)
{
struct efi_handler *handler;
struct efi_disk_obj *diskobj;
- struct disk_partition info;
+ struct udevice *dev;
+ struct disk_part *part;
efi_status_t ret;
- int r;
/* check if this is a block device */
ret = efi_search_protocol(handle, &efi_block_io_guid, &handler);
if (ret != EFI_SUCCESS)
return false;
+ /* find a partition udevice */
diskobj = container_of(handle, struct efi_disk_obj, header);
-
- r = part_get_info(diskobj->desc, diskobj->part, &info);
- if (r)
+ dev = diskobj->dev;
+ if (!dev || dev->driver->id != UCLASS_PARTITION)
return false;
- return !!(info.bootable & PART_EFI_SYSTEM_PARTITION);
+ part = dev_get_uclass_plat(dev);
+
+ return !!(part->gpt_part_info.bootable & PART_EFI_SYSTEM_PARTITION);
}
--
2.33.0
[RFC 11/22] dm: add a hidden link to efi object
This member field in udevice will be used to dereference from udevice
to efi_object (or efi_handle).
Signed-off-by: AKASHI Takahiro
---
include/dm/device.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/dm/device.h b/include/dm/device.h
index 0a9718a5b81a..33b09a836f06 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -190,6 +190,10 @@ struct udevice {
#if CONFIG_IS_ENABLED(DM_DMA)
ulong dma_offset;
#endif
+#if CONFIG_IS_ENABLED(EFI_LOADER)
+ /* link to efi_object */
+ void *efi_obj;
+#endif
};
/**
--
2.33.0
Re: [PATCH] cmd: sf: Support unaligned flash updates with 'sf update'
Hi Frieder,
On Thu, 30 Sept 2021 at 10:20, Frieder Schrempf wrote:
>
> From: Frieder Schrempf
>
> Currently 'sf update' supports only offsets that are aligned to the
> erase block size of the serial flash. Unaligned offsets result in
> something like:
>
> => sf update ${kernel_addr_r} 0x400 ${filesize}
> device 0 offset 0x400, size 0x11f758
> SPI flash failed in erase step
>
> In order to support unaligned updates, we simply read the first full
> block and check only the requested part of the block for changes. If
> necessary, the block is erased, the first (unchanged) part of the block
> is written back together with the second part of the block (updated data).
>
> Signed-off-by: Frieder Schrempf
> ---
> cmd/sf.c | 17 +++--
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
Regards,
Simon
[RFC 10/22] efi_loader: disk: use udevice instead of blk_desc
In most of all usages, we can avoid using blk_desc which is expected
to be data private to the device not be accessed outside device drivers.
Signed-off-by: AKASHI Takahiro
---
lib/efi_loader/efi_disk.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 988907ecb910..dfa6f898d586 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -45,7 +45,7 @@ struct efi_disk_obj {
unsigned int part;
struct efi_simple_file_system_protocol *volume;
lbaint_t offset;
- struct blk_desc *desc;
+ struct udevice *dev; /* TODO: move it to efi_object */
};
/**
@@ -80,14 +80,12 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io
*this,
void *buffer, enum efi_disk_direction direction)
{
struct efi_disk_obj *diskobj;
- struct blk_desc *desc;
int blksz;
int blocks;
unsigned long n;
diskobj = container_of(this, struct efi_disk_obj, ops);
- desc = (struct blk_desc *) diskobj->desc;
- blksz = desc->blksz;
+ blksz = diskobj->media.block_size;
blocks = buffer_size / blksz;
lba += diskobj->offset;
@@ -99,9 +97,9 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io
*this,
return EFI_BAD_BUFFER_SIZE;
if (direction == EFI_DISK_READ)
- n = blk_dread(desc, lba, blocks, buffer);
+ n = blk_read(diskobj->dev, lba, blocks, buffer);
else
- n = blk_dwrite(desc, lba, blocks, buffer);
+ n = blk_write(diskobj->dev, lba, blocks, buffer);
/* We don't do interrupts, so check for timers cooperatively */
efi_timer_check();
@@ -443,7 +441,6 @@ static efi_status_t efi_disk_add_dev(
diskobj->ops = block_io_disk_template;
diskobj->ifname = if_typename;
diskobj->dev_index = dev_index;
- diskobj->desc = desc;
/* Fill in EFI IO Media info (for read/write callbacks) */
diskobj->media.removable_media = desc->removable;
@@ -647,20 +644,22 @@ bool efi_disk_is_system_part(efi_handle_t handle)
{
struct efi_handler *handler;
struct efi_disk_obj *diskobj;
- struct disk_partition info;
+ struct udevice *dev;
+ struct disk_part *part;
efi_status_t ret;
- int r;
/* check if this is a block device */
ret = efi_search_protocol(handle, &efi_block_io_guid, &handler);
if (ret != EFI_SUCCESS)
return false;
+ /* find a partition udevice */
diskobj = container_of(handle, struct efi_disk_obj, header);
-
- r = part_get_info(diskobj->desc, diskobj->part, &info);
- if (r)
+ dev = diskobj->dev;
+ if (!dev || dev->driver->id != UCLASS_PARTITION)
return false;
- return !!(info.bootable & PART_EFI_SYSTEM_PARTITION);
+ part = dev_get_uclass_plat(dev);
+
+ return !!(part->gpt_part_info.bootable & PART_EFI_SYSTEM_PARTITION);
}
--
2.33.0
[RFC 10/22] dm: blk: add read/write interfaces with udevice
In include/blk.h, Simon suggested:
===>
/*
* These functions should take struct udevice instead of struct blk_desc,
* but this is convenient for migration to driver model. Add a 'd' prefix
* to the function operations, so that blk_read(), etc. can be reserved for
* functions with the correct arguments.
*/
unsigned long blk_dread(struct blk_desc *block_dev, lbaint_t start,
lbaint_t blkcnt, void *buffer);
unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start,
lbaint_t blkcnt, const void *buffer);
unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start,
lbaint_t blkcnt);
<===
So new interfaces are provided with this patch.
They are expected to be used everywhere in U-Boot at the end. The exceptions
are block device drivers, partition drivers and efi_disk which should know
details of blk_desc structure.
Signed-off-by: AKASHI Takahiro
---
drivers/block/blk-uclass.c | 91 ++
include/blk.h | 6 +++
2 files changed, 97 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 6ba11a8fa7f7..8fbec8779e1e 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -482,6 +482,97 @@ unsigned long blk_derase(struct blk_desc *block_dev,
lbaint_t start,
return ops->erase(dev, start, blkcnt);
}
+static struct blk_desc *dev_get_blk(struct udevice *dev)
+{
+ struct blk_desc *block_dev;
+
+ switch (device_get_uclass_id(dev)) {
+ case UCLASS_BLK:
+ block_dev = dev_get_uclass_plat(dev);
+ break;
+ case UCLASS_PARTITION:
+ block_dev = dev_get_uclass_plat(dev_get_parent(dev));
+ break;
+ default:
+ block_dev = NULL;
+ break;
+ }
+
+ return block_dev;
+}
+
+unsigned long blk_read(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, void *buffer)
+{
+ struct blk_desc *block_dev;
+ const struct blk_ops *ops;
+ struct disk_part *part;
+ lbaint_t start_in_disk;
+ ulong blks_read;
+
+ block_dev = dev_get_blk(dev);
+ if (!block_dev)
+ return -ENOSYS;
+
+ ops = blk_get_ops(dev);
+ if (!ops->read)
+ return -ENOSYS;
+
+ start_in_disk = start;
+ if (device_get_uclass_id(dev) == UCLASS_PARTITION) {
+ part = dev_get_uclass_plat(dev);
+ start_in_disk += part->gpt_part_info.start;
+ }
+
+ if (blkcache_read(block_dev->if_type, block_dev->devnum,
+ start_in_disk, blkcnt, block_dev->blksz, buffer))
+ return blkcnt;
+ blks_read = ops->read(dev, start, blkcnt, buffer);
+ if (blks_read == blkcnt)
+ blkcache_fill(block_dev->if_type, block_dev->devnum,
+ start_in_disk, blkcnt, block_dev->blksz, buffer);
+
+ return blks_read;
+}
+
+unsigned long blk_write(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, const void *buffer)
+{
+ struct blk_desc *block_dev;
+ const struct blk_ops *ops;
+
+ block_dev = dev_get_blk(dev);
+ if (!block_dev)
+ return -ENOSYS;
+
+ ops = blk_get_ops(dev);
+ if (!ops->write)
+ return -ENOSYS;
+
+ blkcache_invalidate(block_dev->if_type, block_dev->devnum);
+
+ return ops->write(dev, start, blkcnt, buffer);
+}
+
+unsigned long blk_erase(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt)
+{
+ struct blk_desc *block_dev;
+ const struct blk_ops *ops;
+
+ block_dev = dev_get_blk(dev);
+ if (!block_dev)
+ return -ENOSYS;
+
+ ops = blk_get_ops(dev);
+ if (!ops->erase)
+ return -ENOSYS;
+
+ blkcache_invalidate(block_dev->if_type, block_dev->devnum);
+
+ return ops->erase(dev, start, blkcnt);
+}
+
int blk_get_from_parent(struct udevice *parent, struct udevice **devp)
{
struct udevice *dev;
diff --git a/include/blk.h b/include/blk.h
index 3d883eb1db64..f5fdd6633a09 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -284,6 +284,12 @@ unsigned long blk_dwrite(struct blk_desc *block_dev,
lbaint_t start,
unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start,
lbaint_t blkcnt);
+unsigned long blk_read(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, void *buffer);
+unsigned long blk_write(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, const void *buffer);
+unsigned long blk_erase(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt);
/**
* blk_find_device() - Find a block device
*
--
2.33.0
[RFC 09/22] dm: blk: add read/write interfaces with udevice
In include/blk.h, Simon suggested:
===>
/*
* These functions should take struct udevice instead of struct blk_desc,
* but this is convenient for migration to driver model. Add a 'd' prefix
* to the function operations, so that blk_read(), etc. can be reserved for
* functions with the correct arguments.
*/
unsigned long blk_dread(struct blk_desc *block_dev, lbaint_t start,
lbaint_t blkcnt, void *buffer);
unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start,
lbaint_t blkcnt, const void *buffer);
unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start,
lbaint_t blkcnt);
<===
So new interfaces are provided with this patch.
They are expected to be used everywhere in U-Boot at the end. The exceptions
are block device drivers, partition drivers and efi_disk which should know
details of blk_desc structure.
Signed-off-by: AKASHI Takahiro
---
drivers/block/blk-uclass.c | 91 ++
include/blk.h | 6 +++
2 files changed, 97 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 6ba11a8fa7f7..8fbec8779e1e 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -482,6 +482,97 @@ unsigned long blk_derase(struct blk_desc *block_dev,
lbaint_t start,
return ops->erase(dev, start, blkcnt);
}
+static struct blk_desc *dev_get_blk(struct udevice *dev)
+{
+ struct blk_desc *block_dev;
+
+ switch (device_get_uclass_id(dev)) {
+ case UCLASS_BLK:
+ block_dev = dev_get_uclass_plat(dev);
+ break;
+ case UCLASS_PARTITION:
+ block_dev = dev_get_uclass_plat(dev_get_parent(dev));
+ break;
+ default:
+ block_dev = NULL;
+ break;
+ }
+
+ return block_dev;
+}
+
+unsigned long blk_read(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, void *buffer)
+{
+ struct blk_desc *block_dev;
+ const struct blk_ops *ops;
+ struct disk_part *part;
+ lbaint_t start_in_disk;
+ ulong blks_read;
+
+ block_dev = dev_get_blk(dev);
+ if (!block_dev)
+ return -ENOSYS;
+
+ ops = blk_get_ops(dev);
+ if (!ops->read)
+ return -ENOSYS;
+
+ start_in_disk = start;
+ if (device_get_uclass_id(dev) == UCLASS_PARTITION) {
+ part = dev_get_uclass_plat(dev);
+ start_in_disk += part->gpt_part_info.start;
+ }
+
+ if (blkcache_read(block_dev->if_type, block_dev->devnum,
+ start_in_disk, blkcnt, block_dev->blksz, buffer))
+ return blkcnt;
+ blks_read = ops->read(dev, start, blkcnt, buffer);
+ if (blks_read == blkcnt)
+ blkcache_fill(block_dev->if_type, block_dev->devnum,
+ start_in_disk, blkcnt, block_dev->blksz, buffer);
+
+ return blks_read;
+}
+
+unsigned long blk_write(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, const void *buffer)
+{
+ struct blk_desc *block_dev;
+ const struct blk_ops *ops;
+
+ block_dev = dev_get_blk(dev);
+ if (!block_dev)
+ return -ENOSYS;
+
+ ops = blk_get_ops(dev);
+ if (!ops->write)
+ return -ENOSYS;
+
+ blkcache_invalidate(block_dev->if_type, block_dev->devnum);
+
+ return ops->write(dev, start, blkcnt, buffer);
+}
+
+unsigned long blk_erase(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt)
+{
+ struct blk_desc *block_dev;
+ const struct blk_ops *ops;
+
+ block_dev = dev_get_blk(dev);
+ if (!block_dev)
+ return -ENOSYS;
+
+ ops = blk_get_ops(dev);
+ if (!ops->erase)
+ return -ENOSYS;
+
+ blkcache_invalidate(block_dev->if_type, block_dev->devnum);
+
+ return ops->erase(dev, start, blkcnt);
+}
+
int blk_get_from_parent(struct udevice *parent, struct udevice **devp)
{
struct udevice *dev;
diff --git a/include/blk.h b/include/blk.h
index 3d883eb1db64..f5fdd6633a09 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -284,6 +284,12 @@ unsigned long blk_dwrite(struct blk_desc *block_dev,
lbaint_t start,
unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start,
lbaint_t blkcnt);
+unsigned long blk_read(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, void *buffer);
+unsigned long blk_write(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, const void *buffer);
+unsigned long blk_erase(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt);
/**
* blk_find_device() - Find a block device
*
--
2.33.0
[RFC 09/22] dm: blk: add a device-probe hook for scanning disk partitions
Now that all the block device drivers have enable a probe hook, we will
call blk_create_partitions() to enumerate all the partitions and create
associated udevices when a block device is detected.
Signed-off-by: AKASHI Takahiro
---
drivers/block/blk-uclass.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index dd7f3c0fe31e..6ba11a8fa7f7 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -741,11 +741,25 @@ static int blk_post_probe(struct udevice *dev)
struct blk_desc *desc = dev_get_uclass_plat(dev);
part_init(desc);
+
+ if (desc->part_type != PART_TYPE_UNKNOWN &&
+ blk_create_partitions(dev))
+ debug("*** creating partitions failed\n");
}
return 0;
}
+static int blk_part_post_probe(struct udevice *dev)
+{
+ /*
+* TODO:
+* If we call blk_creat_partitions() here, it would allow for
+* "partitions in a partition".
+*/
+ return 0;
+}
+
UCLASS_DRIVER(blk) = {
.id = UCLASS_BLK,
.name = "blk",
@@ -821,6 +835,7 @@ U_BOOT_DRIVER(blk_partition) = {
UCLASS_DRIVER(partition) = {
.id = UCLASS_PARTITION,
+ .post_probe = blk_part_post_probe,
.per_device_plat_auto = sizeof(struct disk_part),
.name = "partition",
};
--
2.33.0
[RFC 08/22] dm: blk: add a device-probe hook for scanning disk partitions
Now that all the block device drivers have enable a probe hook, we will
call blk_create_partitions() to enumerate all the partitions and create
associated udevices when a block device is detected.
Signed-off-by: AKASHI Takahiro
---
drivers/block/blk-uclass.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index dd7f3c0fe31e..6ba11a8fa7f7 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -741,11 +741,25 @@ static int blk_post_probe(struct udevice *dev)
struct blk_desc *desc = dev_get_uclass_plat(dev);
part_init(desc);
+
+ if (desc->part_type != PART_TYPE_UNKNOWN &&
+ blk_create_partitions(dev))
+ debug("*** creating partitions failed\n");
}
return 0;
}
+static int blk_part_post_probe(struct udevice *dev)
+{
+ /*
+* TODO:
+* If we call blk_creat_partitions() here, it would allow for
+* "partitions in a partition".
+*/
+ return 0;
+}
+
UCLASS_DRIVER(blk) = {
.id = UCLASS_BLK,
.name = "blk",
@@ -821,6 +835,7 @@ U_BOOT_DRIVER(blk_partition) = {
UCLASS_DRIVER(partition) = {
.id = UCLASS_PARTITION,
+ .post_probe = blk_part_post_probe,
.per_device_plat_auto = sizeof(struct disk_part),
.name = "partition",
};
--
2.33.0
[RFC 08/22] dm: blk: add UCLASS_PARTITION
UCLASS_PARTITION device will be created as a child node of
UCLASS_BLK device.
Signed-off-by: AKASHI Takahiro
---
drivers/block/blk-uclass.c | 111 +
include/blk.h | 9 +++
include/dm/uclass-id.h | 1 +
3 files changed, 121 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 83682dcc181a..dd7f3c0fe31e 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -695,6 +696,44 @@ int blk_unbind_all(int if_type)
return 0;
}
+int blk_create_partitions(struct udevice *parent)
+{
+ int part, count;
+ struct blk_desc *desc = dev_get_uclass_plat(parent);
+ struct disk_partition info;
+ struct disk_part *part_data;
+ char devname[32];
+ struct udevice *dev;
+ int ret;
+
+ if (!CONFIG_IS_ENABLED(PARTITIONS) ||
+ !CONFIG_IS_ENABLED(HAVE_BLOCK_DEVICE))
+ return 0;
+
+ /* Add devices for each partition */
+ for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
+ if (part_get_info(desc, part, &info))
+ continue;
+ snprintf(devname, sizeof(devname), "%s:%d", parent->name,
+part);
+
+ ret = device_bind_driver(parent, "blk_partition",
+strdup(devname), &dev);
+ if (ret)
+ return ret;
+
+ part_data = dev_get_uclass_plat(dev);
+ part_data->partnum = part;
+ part_data->gpt_part_info = info;
+ count++;
+
+ device_probe(dev);
+ }
+ debug("%s: %d partitions found in %s\n", __func__, count, parent->name);
+
+ return 0;
+}
+
static int blk_post_probe(struct udevice *dev)
{
if (IS_ENABLED(CONFIG_PARTITIONS) &&
@@ -713,3 +752,75 @@ UCLASS_DRIVER(blk) = {
.post_probe = blk_post_probe,
.per_device_plat_auto = sizeof(struct blk_desc),
};
+
+static ulong blk_part_read(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, void *buffer)
+{
+ struct udevice *parent;
+ struct disk_part *part;
+ const struct blk_ops *ops;
+
+ parent = dev_get_parent(dev);
+ ops = blk_get_ops(parent);
+ if (!ops->read)
+ return -ENOSYS;
+
+ part = dev_get_uclass_plat(dev);
+ start += part->gpt_part_info.start;
+
+ return ops->read(parent, start, blkcnt, buffer);
+}
+
+static ulong blk_part_write(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, const void *buffer)
+{
+ struct udevice *parent;
+ struct disk_part *part;
+ const struct blk_ops *ops;
+
+ parent = dev_get_parent(dev);
+ ops = blk_get_ops(parent);
+ if (!ops->write)
+ return -ENOSYS;
+
+ part = dev_get_uclass_plat(dev);
+ start += part->gpt_part_info.start;
+
+ return ops->write(parent, start, blkcnt, buffer);
+}
+
+static ulong blk_part_erase(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt)
+{
+ struct udevice *parent;
+ struct disk_part *part;
+ const struct blk_ops *ops;
+
+ parent = dev_get_parent(dev);
+ ops = blk_get_ops(parent);
+ if (!ops->erase)
+ return -ENOSYS;
+
+ part = dev_get_uclass_plat(dev);
+ start += part->gpt_part_info.start;
+
+ return ops->erase(parent, start, blkcnt);
+}
+
+static const struct blk_ops blk_part_ops = {
+ .read = blk_part_read,
+ .write = blk_part_write,
+ .erase = blk_part_erase,
+};
+
+U_BOOT_DRIVER(blk_partition) = {
+ .name = "blk_partition",
+ .id = UCLASS_PARTITION,
+ .ops= &blk_part_ops,
+};
+
+UCLASS_DRIVER(partition) = {
+ .id = UCLASS_PARTITION,
+ .per_device_plat_auto = sizeof(struct disk_part),
+ .name = "partition",
+};
diff --git a/include/blk.h b/include/blk.h
index 19bab081c2cd..3d883eb1db64 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -366,6 +366,15 @@ int blk_create_devicef(struct udevice *parent, const char
*drv_name,
const char *name, int if_type, int devnum, int blksz,
lbaint_t lba, struct udevice **devp);
+/**
+ * blk_create_partitions - Create block devices for disk partitions
+ *
+ * Create UCLASS_PARTITION udevices for each of disk partitions in @parent
+ *
+ * @parent:Whole disk device
+ */
+int blk_create_partitions(struct udevice *parent);
+
/**
* blk_unbind_all() - Unbind all device of the given interface type
*
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index e7edd409f307..30892d01ce13 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -80,6
[RFC 07/22] dm: blk: add UCLASS_PARTITION
UCLASS_PARTITION device will be created as a child node of
UCLASS_BLK device.
Signed-off-by: AKASHI Takahiro
---
drivers/block/blk-uclass.c | 111 +
include/blk.h | 9 +++
include/dm/uclass-id.h | 1 +
3 files changed, 121 insertions(+)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 83682dcc181a..dd7f3c0fe31e 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -695,6 +696,44 @@ int blk_unbind_all(int if_type)
return 0;
}
+int blk_create_partitions(struct udevice *parent)
+{
+ int part, count;
+ struct blk_desc *desc = dev_get_uclass_plat(parent);
+ struct disk_partition info;
+ struct disk_part *part_data;
+ char devname[32];
+ struct udevice *dev;
+ int ret;
+
+ if (!CONFIG_IS_ENABLED(PARTITIONS) ||
+ !CONFIG_IS_ENABLED(HAVE_BLOCK_DEVICE))
+ return 0;
+
+ /* Add devices for each partition */
+ for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
+ if (part_get_info(desc, part, &info))
+ continue;
+ snprintf(devname, sizeof(devname), "%s:%d", parent->name,
+part);
+
+ ret = device_bind_driver(parent, "blk_partition",
+strdup(devname), &dev);
+ if (ret)
+ return ret;
+
+ part_data = dev_get_uclass_plat(dev);
+ part_data->partnum = part;
+ part_data->gpt_part_info = info;
+ count++;
+
+ device_probe(dev);
+ }
+ debug("%s: %d partitions found in %s\n", __func__, count, parent->name);
+
+ return 0;
+}
+
static int blk_post_probe(struct udevice *dev)
{
if (IS_ENABLED(CONFIG_PARTITIONS) &&
@@ -713,3 +752,75 @@ UCLASS_DRIVER(blk) = {
.post_probe = blk_post_probe,
.per_device_plat_auto = sizeof(struct blk_desc),
};
+
+static ulong blk_part_read(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, void *buffer)
+{
+ struct udevice *parent;
+ struct disk_part *part;
+ const struct blk_ops *ops;
+
+ parent = dev_get_parent(dev);
+ ops = blk_get_ops(parent);
+ if (!ops->read)
+ return -ENOSYS;
+
+ part = dev_get_uclass_plat(dev);
+ start += part->gpt_part_info.start;
+
+ return ops->read(parent, start, blkcnt, buffer);
+}
+
+static ulong blk_part_write(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt, const void *buffer)
+{
+ struct udevice *parent;
+ struct disk_part *part;
+ const struct blk_ops *ops;
+
+ parent = dev_get_parent(dev);
+ ops = blk_get_ops(parent);
+ if (!ops->write)
+ return -ENOSYS;
+
+ part = dev_get_uclass_plat(dev);
+ start += part->gpt_part_info.start;
+
+ return ops->write(parent, start, blkcnt, buffer);
+}
+
+static ulong blk_part_erase(struct udevice *dev, lbaint_t start,
+ lbaint_t blkcnt)
+{
+ struct udevice *parent;
+ struct disk_part *part;
+ const struct blk_ops *ops;
+
+ parent = dev_get_parent(dev);
+ ops = blk_get_ops(parent);
+ if (!ops->erase)
+ return -ENOSYS;
+
+ part = dev_get_uclass_plat(dev);
+ start += part->gpt_part_info.start;
+
+ return ops->erase(parent, start, blkcnt);
+}
+
+static const struct blk_ops blk_part_ops = {
+ .read = blk_part_read,
+ .write = blk_part_write,
+ .erase = blk_part_erase,
+};
+
+U_BOOT_DRIVER(blk_partition) = {
+ .name = "blk_partition",
+ .id = UCLASS_PARTITION,
+ .ops= &blk_part_ops,
+};
+
+UCLASS_DRIVER(partition) = {
+ .id = UCLASS_PARTITION,
+ .per_device_plat_auto = sizeof(struct disk_part),
+ .name = "partition",
+};
diff --git a/include/blk.h b/include/blk.h
index 19bab081c2cd..3d883eb1db64 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -366,6 +366,15 @@ int blk_create_devicef(struct udevice *parent, const char
*drv_name,
const char *name, int if_type, int devnum, int blksz,
lbaint_t lba, struct udevice **devp);
+/**
+ * blk_create_partitions - Create block devices for disk partitions
+ *
+ * Create UCLASS_PARTITION udevices for each of disk partitions in @parent
+ *
+ * @parent:Whole disk device
+ */
+int blk_create_partitions(struct udevice *parent);
+
/**
* blk_unbind_all() - Unbind all device of the given interface type
*
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index e7edd409f307..30892d01ce13 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -80,6
[RFC 07/22] block: ide: call device_probe() after scanning
Every time an ide bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
drivers/block/ide.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index c99076c6f45d..31aaed09ab70 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -1151,6 +1151,12 @@ static int ide_probe(struct udevice *udev)
blksz, size, &blk_dev);
if (ret)
return ret;
+
+ ret = device_probe(blk_dev);
+ if (ret) {
+ device_unbind(blk_dev);
+ return ret;
+ }
}
}
--
2.33.0
[RFC 06/22] sata: call device_probe() after scanning
Every time a sata bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
drivers/ata/dwc_ahsata.c | 10 ++
drivers/ata/fsl_sata.c | 11 +++
drivers/ata/sata_mv.c| 9 +
drivers/ata/sata_sil.c | 12
4 files changed, 42 insertions(+)
diff --git a/drivers/ata/dwc_ahsata.c b/drivers/ata/dwc_ahsata.c
index 6d42548087b3..6a51c70d1170 100644
--- a/drivers/ata/dwc_ahsata.c
+++ b/drivers/ata/dwc_ahsata.c
@@ -1026,6 +1026,16 @@ int dwc_ahsata_scan(struct udevice *dev)
return ret;
}
+ ret = device_probe(bdev);
+ if (ret < 0) {
+ debug("Can't probe\n");
+ /* TODO: undo create */
+
+ device_unbind(bdev);
+
+ return ret;
+ }
+
return 0;
}
diff --git a/drivers/ata/fsl_sata.c b/drivers/ata/fsl_sata.c
index e44db0a37458..346e9298b4c5 100644
--- a/drivers/ata/fsl_sata.c
+++ b/drivers/ata/fsl_sata.c
@@ -982,6 +982,17 @@ static int fsl_ata_probe(struct udevice *dev)
failed_number++;
continue;
}
+
+ ret = device_probe(bdev);
+ if (ret < 0) {
+ debug("Can't probe\n");
+ ret = fsl_unbind_device(blk);
+ if (ret)
+ return ret;
+
+ failed_number++;
+ continue;
+ }
}
if (failed_number == nr_ports)
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 003222d47be6..09b735779ebf 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -1099,6 +1099,15 @@ static int sata_mv_probe(struct udevice *dev)
continue;
}
+ ret = device_probe(bdev);
+ if (ret < 0) {
+ debug("Can't probe\n");
+ /* TODO: undo create */
+
+ device_unbind(bdev);
+ continue;
+ }
+
/* If we got here, the current SATA port was probed
* successfully, so set the probe status to successful.
*/
diff --git a/drivers/ata/sata_sil.c b/drivers/ata/sata_sil.c
index dda712f42cb2..295f7ca72303 100644
--- a/drivers/ata/sata_sil.c
+++ b/drivers/ata/sata_sil.c
@@ -864,6 +864,18 @@ static int sil_pci_probe(struct udevice *dev)
failed_number++;
continue;
}
+
+ ret = device_probe(bdev);
+ if (ret < 0) {
+ debug("Can't probe\n");
+ ret = sil_unbind_device(blk);
+ device_unbind(bdev);
+ if (ret)
+ return ret;
+
+ failed_number++;
+ continue;
+ }
}
if (failed_number == sata_info.maxport)
--
2.33.0
[RFC 06/22] block: ide: call device_probe() after scanning
Every time an ide bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
drivers/block/ide.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index c99076c6f45d..31aaed09ab70 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -1151,6 +1151,12 @@ static int ide_probe(struct udevice *udev)
blksz, size, &blk_dev);
if (ret)
return ret;
+
+ ret = device_probe(blk_dev);
+ if (ret) {
+ device_unbind(blk_dev);
+ return ret;
+ }
}
}
--
2.33.0
[RFC 05/22] sata: call device_probe() after scanning
Every time a sata bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
drivers/ata/dwc_ahsata.c | 10 ++
drivers/ata/fsl_sata.c | 11 +++
drivers/ata/sata_mv.c| 9 +
drivers/ata/sata_sil.c | 12
4 files changed, 42 insertions(+)
diff --git a/drivers/ata/dwc_ahsata.c b/drivers/ata/dwc_ahsata.c
index 6d42548087b3..6a51c70d1170 100644
--- a/drivers/ata/dwc_ahsata.c
+++ b/drivers/ata/dwc_ahsata.c
@@ -1026,6 +1026,16 @@ int dwc_ahsata_scan(struct udevice *dev)
return ret;
}
+ ret = device_probe(bdev);
+ if (ret < 0) {
+ debug("Can't probe\n");
+ /* TODO: undo create */
+
+ device_unbind(bdev);
+
+ return ret;
+ }
+
return 0;
}
diff --git a/drivers/ata/fsl_sata.c b/drivers/ata/fsl_sata.c
index e44db0a37458..346e9298b4c5 100644
--- a/drivers/ata/fsl_sata.c
+++ b/drivers/ata/fsl_sata.c
@@ -982,6 +982,17 @@ static int fsl_ata_probe(struct udevice *dev)
failed_number++;
continue;
}
+
+ ret = device_probe(bdev);
+ if (ret < 0) {
+ debug("Can't probe\n");
+ ret = fsl_unbind_device(blk);
+ if (ret)
+ return ret;
+
+ failed_number++;
+ continue;
+ }
}
if (failed_number == nr_ports)
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 003222d47be6..09b735779ebf 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -1099,6 +1099,15 @@ static int sata_mv_probe(struct udevice *dev)
continue;
}
+ ret = device_probe(bdev);
+ if (ret < 0) {
+ debug("Can't probe\n");
+ /* TODO: undo create */
+
+ device_unbind(bdev);
+ continue;
+ }
+
/* If we got here, the current SATA port was probed
* successfully, so set the probe status to successful.
*/
diff --git a/drivers/ata/sata_sil.c b/drivers/ata/sata_sil.c
index dda712f42cb2..295f7ca72303 100644
--- a/drivers/ata/sata_sil.c
+++ b/drivers/ata/sata_sil.c
@@ -864,6 +864,18 @@ static int sil_pci_probe(struct udevice *dev)
failed_number++;
continue;
}
+
+ ret = device_probe(bdev);
+ if (ret < 0) {
+ debug("Can't probe\n");
+ ret = sil_unbind_device(blk);
+ device_unbind(bdev);
+ if (ret)
+ return ret;
+
+ failed_number++;
+ continue;
+ }
}
if (failed_number == sata_info.maxport)
--
2.33.0
[RFC 05/22] nvme: call device_probe() after scanning
Every time a nvme bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
drivers/nvme/nvme.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index f6465ea7f482..975bbc6dc3b7 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -909,6 +909,12 @@ static int nvme_probe(struct udevice *udev)
-1, 512, 0, &ns_udev);
if (ret)
goto free_id;
+
+ ret = device_probe(ns_udev);
+ if (ret) {
+ device_unbind(ns_udev);
+ goto free_id;
+ }
}
free(id);
--
2.33.0
[RFC 04/22] nvme: call device_probe() after scanning
Every time a nvme bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
drivers/nvme/nvme.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index f6465ea7f482..975bbc6dc3b7 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -909,6 +909,12 @@ static int nvme_probe(struct udevice *udev)
-1, 512, 0, &ns_udev);
if (ret)
goto free_id;
+
+ ret = device_probe(ns_udev);
+ if (ret) {
+ device_unbind(ns_udev);
+ goto free_id;
+ }
}
free(id);
--
2.33.0
[RFC 04/22] mmc: call device_probe() after scanning
Every time a mmc bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
drivers/mmc/mmc-uclass.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 3ee92d03ca23..07b5c1736439 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -418,6 +418,13 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const
struct mmc_config *cfg)
bdesc->part_type = cfg->part_type;
mmc->dev = dev;
mmc->user_speed_mode = MMC_MODES_END;
+
+ ret = device_probe(dev);
+ if (ret) {
+ device_unbind(dev);
+ return ret;
+ }
+
return 0;
}
--
2.33.0
[RFC 03/22] usb: storage: call device_probe() after scanning
Every time a usb bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
common/usb_storage.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/common/usb_storage.c b/common/usb_storage.c
index 946c6b2b323a..5f294f17491f 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -239,6 +239,12 @@ static int usb_stor_probe_device(struct usb_device *udev)
if (ret)
return ret;
}
+
+ ret = device_probe(dev);
+ if (ret) {
+ device_unbind(dev);
+ return ret;
+ }
}
#else
/* We don't have space to even probe if we hit the maximum */
--
2.33.0
[RFC 03/22] mmc: call device_probe() after scanning
Every time a mmc bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
drivers/mmc/mmc-uclass.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 3ee92d03ca23..07b5c1736439 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -418,6 +418,13 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const
struct mmc_config *cfg)
bdesc->part_type = cfg->part_type;
mmc->dev = dev;
mmc->user_speed_mode = MMC_MODES_END;
+
+ ret = device_probe(dev);
+ if (ret) {
+ device_unbind(dev);
+ return ret;
+ }
+
return 0;
}
--
2.33.0
[RFC 02/22] usb: storage: call device_probe() after scanning
Every time a usb bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
common/usb_storage.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/common/usb_storage.c b/common/usb_storage.c
index 946c6b2b323a..5f294f17491f 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -239,6 +239,12 @@ static int usb_stor_probe_device(struct usb_device *udev)
if (ret)
return ret;
}
+
+ ret = device_probe(dev);
+ if (ret) {
+ device_unbind(dev);
+ return ret;
+ }
}
#else
/* We don't have space to even probe if we hit the maximum */
--
2.33.0
[RFC 01/22] scsi: call device_probe() after scanning
Every time a scsi bus/port is scanned and a new block device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
drivers/scsi/scsi.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index d93d24192853..4865b5a86fd5 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -595,6 +595,16 @@ static int do_scsi_scan_one(struct udevice *dev, int id,
int lun, bool verbose)
ata_swap_buf_le16((u16 *)&bdesc->revision, sizeof(bd.revision)
/ 2);
}
+ ret = device_probe(bdev);
+ if (ret < 0) {
+ debug("Can't probe\n");
+ /* TODO: undo create */
+
+ ret = device_unbind(bdev);
+
+ return ret;
+ }
+
if (verbose) {
printf(" Device %d: ", bdesc->devnum);
dev_print(bdesc);
--
2.33.0
[RFC 02/22] scsi: call device_probe() after scanning
Every time a scsi bus/port is scanned and a new block device is detected,
we want to call device_probe() as it will give us a chance to run additional
post-processings for some purposes.
In particular, support for creating partitions on a device will be added.
Signed-off-by: AKASHI Takahiro
---
drivers/scsi/scsi.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index d93d24192853..4865b5a86fd5 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -595,6 +595,16 @@ static int do_scsi_scan_one(struct udevice *dev, int id,
int lun, bool verbose)
ata_swap_buf_le16((u16 *)&bdesc->revision, sizeof(bd.revision)
/ 2);
}
+ ret = device_probe(bdev);
+ if (ret < 0) {
+ debug("Can't probe\n");
+ /* TODO: undo create */
+
+ ret = device_unbind(bdev);
+
+ return ret;
+ }
+
if (verbose) {
printf(" Device %d: ", bdesc->devnum);
dev_print(bdesc);
--
2.33.0
[RFC 01/22] part: call part_init() in blk_get_device_by_str() only for MMC
In blk_get_device_by_str(), the comment says: "Updates the partition table for the specified hw partition." Since hw partition is supported only on MMC, it makes no sense to do so for other devices. Signed-off-by: AKASHI Takahiro --- disk/part.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/disk/part.c b/disk/part.c index a6a8f7052bd3..b330103a5bc0 100644 --- a/disk/part.c +++ b/disk/part.c @@ -427,7 +427,8 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str, * Always should be done, otherwise hw partition 0 will return stale * data after displaying a non-zero hw partition. */ - part_init(*dev_desc); + if ((*dev_desc)->if_type == IF_TYPE_MMC) + part_init(*dev_desc); #endif cleanup: -- 2.33.0
[RFC 00/22] efi_loader: more tightly integrate UEFI disks to device model
The purpose of this RPC is to reignite the discussion about how UEFI
subystem would best be integrated into U-Boot device model.
In the past, I poposed a couple of patch series, the latest one[1],
while Heinrich revealed his idea[2], and the approach taken here is
something between them, with a focus on block device handlings.
# The code is a PoC and not well tested yet.
Disks in UEFI world:
In general in UEFI world, accessing to any device is performed through
a 'protocol' interface which are installed to (or associated with) the device's
UEFI handle (or an opaque pointer to UEFI object data). Protocols are
implemented by either the UEFI system itself or UEFI drivers.
For block IO's, it is a device which has EFI_BLOCK_IO_PROTOCOL (efi_disk
hereafter). Currently, every efi_disk may have one of two origins:
a.U-Boot's block devices or related partitions
(lib/efi_loader/efi_disk.c)
b.UEFI objects which are implemented as a block device by UEFI drivers.
(lib/efi_driver/efi_block_device.c)
All the efi_diskss as (a) will be enumelated and created only once at UEFI
subsystem initialization (efi_disk_register()), which is triggered by
first executing one of UEFI-related U-Boot commands, like "bootefi",
"setenv -e" or "efidebug".
EFI_BLOCK_IO_PROTOCOL is implemented by UEFI system using blk_desc(->ops)
in the corresponding udevice(UCLASS_BLK).
On the other hand, efi_disk as (b) will be created each time UEFI boot
services' connect_controller() is executed in UEFI app which, as a (device)
controller, gives the method to access the device's data,
ie. EFI_BLOCK_IO_PROTOCOL.
>>> more details >>>
Internally, connect_controller() search for UEFI driver that can support
this controller/protocol, 'efi_block' driver(UCLASS_EFI) in this case,
then calls the driver's 'bind' interface, which eventually installs
the controller's EFI_BLOCK_IO_PROTOCOL to efi_disk object.
'efi_block' driver also create a corresponding udevice(UCLASS_BLK) for
* creating additional partitions efi_disk's, and
* supporting a file system (EFI_SIMPLE_FILE_SYSTEM_PROTOCOL) on it.
<<< <<<
Issues:
===
1. While an efi_disk represents a device equally for either a whole disk
or a partition in UEFI world, the device model treats only a whole
disk as a real block device or udevice(UCLASS_BLK).
2. efi_disk holds and makes use of "blk_desc" data even though blk_desc
in plat_data is supposed to be private and not to be accessed outside
the device model.
# This issue, though, exists for all the implmenetation of U-Boot
# file systems as well.
For efi_disk(a),
3. A block device can be enumelated dynamically by 'scanning' a device bus
in U-Boot, but UEFI subsystem is not able to update efi_disks accordingly.
For examples,
=> scsi rescan; efidebug devices
=> usb start; efidebug devices ... (A)
(A) doesn't show any usb devices detected.
=> scsi rescan; efidebug boot add -b 0 TEST scsi 0:1 ...
=> scsi rescan ... (B)
=> bootefi bootmgr ... (C)
(C) may de-reference a bogus blk_desc pointer which has been freed by (B).
(Please note that "scsi rescan" removes all udevices/blk_desc and then
re-create them even if nothing is changed on a bus.)
For efi_disk(b),
4. A controller (handle), combined with efi_block driver, has no
corresponding udevice as a parent of efi_disks in DM tree, unlike, say,
a scsi controller, even though it provides methods for block io perations.
5. There is no way supported to remove efi_disk's even after
disconnect_controller() is called.
My approach in this RFC:
Due to functional differences in semantics, it would be difficult
to identify "udevice" structure as a handle in UEFI world. Instead, we will
have to somehow maintain a relationship between a udevice and a handle.
1-1. add a dedicated uclass, UCLASS_PARTITION, for partitions
Currently, the uclass for paritions is not a UCLASS_BLK.
It can be possible to define partitions as UCLASS_BLK
(with IF_TYPE_PARTION?), but
I'm afraid that it may introduce some chaos since udevice(UCLASS_BLK)
is tightly coupled with 'struct blk_desc' data which is still used
as a "structure to a whole disk" in a lot of interfaces.
(I hope that you understand what it means.)
In DM tree, a UCLASS_PARTITON instance has a UCLASS_BLK parent:
For instance,
UCLASS_SCSI --- UCLASS_BLK --- UCLASS_PARTITION
(IF_TYPE_SCSI)|
+- struct blk_desc +- struct disk_part
+- scsi_blk_ops +- blk_part_ops
1-2. create partition udevices in the context of device_probe()
part_init() is already called in blk_post_probe(). See the commit
d0851c893706 ("blk: Call part_init() in the post_probe() method").
Why not enumelate partitions as well in there.
2. add new block access interfaces, which takes "udevice" as a target device,
in U-Boot and use those functions to im
Re: [PATCH u-boot-marvell v3 00/39] kwboot higher baudrate
Hi Pali, On 30.09.21 20:14, Pali Rohár wrote: Hello! Could you test or review this patch series? It's on my list. It is a big improvement for kwboot as it allows to transfer u-boot over uart into mvebu platforms much faster. Very much appreciated. I'll try to find some time today to review it and perhaps push it to next soon. Thanks, Stefan On Friday 24 September 2021 23:06:37 Marek Behún wrote: From: Marek Behún Hello Stefan and others, here's v3 of series adding support for booting Marvell platforms via UART (those bootable with kwboot) at higher baudrates. Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than 115200 Bd. The user can direct kwboot to use higher baudrate via the -B option. (BTW this option was there before but did not work with the -b option.) Only the payload part of the KWB image is uploaded at this higher baudrate. The header part is still uploaded at 115200 Bd, since the code that changes baudrate is injected into header as a binary extension. (The payload part makes up majority of the binary, though. On Turris Omnia the payload currently makes ~87%.) The series also contains various other fixes, refactors and improvements of the code, upon which the main change is done. Marek & Pali Changes since v2: - fixed integer overflow in patch 15 - fixed commit title in patch 32 Changes since v1: - fixed uploading of older kwb images, now all kwb images should be able to upload at faster baudrate - fixed injecting code that changes baudrate back - various other fixes and refactors, the best way to compare with v1 is to apply v1 and v2 separately and compare the resulting kwboot.c Marek Behún (19): tools: kwbimage: Fix printf format warning tools: kwboot: Fix buffer overflow in kwboot_terminal() tools: kwboot: Make the quit sequence buffer const tools: kwboot: Refactor and fix writing buffer tools: kwboot: Fix comparison of integers with different size tools: kwboot: Use a function to check whether received byte is a Xmodem reply tools: kwboot: Print new line after SPL output tools: kwboot: Allow greater timeout when executing header code tools: kwboot: Prevent waiting indefinitely if no xmodem reply is received tools: kwbimage: Simplify iteration over version 1 optional headers tools: kwbimage: Refactor image_version() tools: kwbimage: Refactor kwbimage header size determination tools: kwboot: Explicitly check against size of struct main_hdr_v1 tools: kwboot: Check whether baudrate was set to requested value tools: kwboot: Cosmetic fix tools: kwboot: Avoid code repetition in kwboot_img_patch() tools: kwboot: Update file header doc/kwboot.1: Update man page MAINTAINERS: Add entry for kwbimage / kwboot tools Pali Rohár (20): tools: kwboot: Print version information header tools: kwboot: Fix kwboot_xm_sendblock() function when kwboot_tty_recv() fails tools: kwboot: Fix return type of kwboot_xm_makeblock() function tools: kwboot: Fix printing progress tools: kwboot: Print newline on error when progress was not completed tools: kwboot: Split sending image into header and data stages tools: kwboot: Allow non-xmodem text output from BootROM only in a specific case tools: kwboot: Properly finish xmodem transfer tools: kwboot: Always call kwboot_img_patch_hdr() tools: kwboot: Don't patch image header if signed tools: kwboot: Patch source address in image header tools: kwboot: Patch destination address to DDR area for SPI image tools: kwbimage: Update comments describing kwbimage v1 structures tools: kwboot: Round up header size to 128 B when patching tools: kwboot: Support higher baudrates when booting via UART tools: kwboot: Allow any baudrate on Linux tools: kwboot: Fix initializing tty device tools: kwboot: Disable tty interbyte timeout tools: kwboot: Disable non-blocking mode tools: kwboot: Add Pali and Marek as authors MAINTAINERS | 10 + doc/kwboot.1 | 60 ++- tools/kwbimage.c | 130 ++--- tools/kwbimage.h | 99 +++- tools/kwboot.c| 1197 +++-- tools/termios_linux.h | 189 +++ 6 files changed, 1385 insertions(+), 300 deletions(-) create mode 100644 tools/termios_linux.h -- 2.32.0 Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: Driver model at UEFI runtime
Am 2021-10-01 00:20, schrieb François Ozog: Le ven. 1 oct. 2021 à 00:00, Michael Walle a écrit : With SystemReady, DT from distros are ignored. Err? Is this really true, I know about some incompatible changes to the device tree which prevents you from using usb (or even a kernel panic) with the imx8mm and I know that on the ls1028a flexspi wont work if the devicetree doesn't match the kernel. I.e. if you have a device tree from kernel 5.14 you cannot use it on 5.10. If you have one from 5.10 you cannot use it on a later kernel. What you describe is the situation we want to avoid and that comes from years of tinkering. how is that tinkering? That means once you have a device tree, it is set in stone. which isn't reality, sorry. Consider the ls1028a and the flexspi "bug". For the 5.10 kernel/dtb there was a wrong clock connected to the flexspi device. There wasn't even a driver for the correct clock. The clock could have been described even though there was no Linux driver. DT is there to describe HW, not the capacity of a particular OS or boot firmware to deal with it. You're missing my point. It's not about what would be the perfect scenario. But what actually happens in reality. And unfortunately, it happens, so you have to deal with devicetree incompatibilies. Just ignoring this and keeping just one devicetree in the firmware is doomed to fail sooner or later. We have an example of firmware provided hardware description that works well (Ok: 99% of the time): ACPI. Mh, I can't really comment on this as I am not familiar with ACPI. But judging by all the linux acpi fixups and bios incompatiblies, my gut tells me that it doesn't work _that_ well ;) We also are 100% sure that the current situation is messy, hairy, buggy but familiar. [..] Regarding the imx8mm usb error, apparently the node was renamed. If you're using older kernels with newer dtbs, the kernel oopes. If you're using a newer kernel with older dtbs, USB doesn't get probed. (Although I was told that there is a "fix" for this, that is, both node names are tried.) I keep seeing those issues about node renames or compatible string changes. That's the tinkering I am talking about. There is no in-kernel ABI but Linus bang heads if anyone touches userland-kernel ABI inappropriately. As DT is mostly handled in-kernel, people treat DT as no-ABI. But it is part of firmware-kernel ABI. Until we properly organize this firmware-kernel ABI, the problem you describe and more will continue. Sure, but until you reaches that point (I predict it wont happen soon or at all) you'll have to deal with per kernel devicetrees. Just saying, we are just providing one devicetree supplied by the firmware just doesn't work with the current situation. So IMHO if SystemReady is really "it just works", then you have to consider this. And so far, it seems all SystemReady certified boards just throw the u-boot devicetree at linux and hope for the best. SystemReady is not meant to be all boards, starting now and mandatory. It is a selected of boards for which everyone in the value chain is willing to spend the energy to solve the problem as if we were living in a perfect world. And here I am, trying to find a solution to a real problem I'm facing with my board and the systemready cerification. But all I'm hearing is that it should work the way linaro/ARM have in mind, but it clearly doesn't. Now the DT lifecycle before the firmware-kernel ABI also needs to be specified so that we properly handle hats, capes, SoMs, carrier boards... https://developer.arm.com/architectures/system-architectures/arm-systemready/ir lists a number of certified boards and the list is going to grow significantly. And the sl28 board will likely be there soon, too. On those boards, you will be able to boot any kernel. Actually I don't think so, because you might hit the imx8mm bug, too. May just be lucky by the devicetree/kernel combination. The image I have in mind with OS provided DT is: as a French driver, my DT says that the steering wheel is on the left hand side of the car. Shall I whine about the cars in England that do not comply to my DT? or accept to use the car provided DT? The situation is comfortable for tinkerers, but not sustainable. It is a matter of organization and not technology to solve the problems you describe. Mh, I'm not sure I understand what you're trying to tell me. The distribution also provides you the kernel, why shouldn't it provide devicetree which exactly matches this kernel and was also tested against. Because the kernel has no clue which hats, capes has been added for instance. And the same might be true for the firmware as pointed out before. The kernel provided DTB works fine when the firmware builder and OS builders are the same. Ehh? It is just the other way around. The distro supplies the kernel and thus it also have the corresponding devicetrees for this particular kernel.
Re: Driver model at UEFI runtime
Le ven. 1 oct. 2021 à 00:00, Michael Walle a écrit : > > With SystemReady, DT from distros are ignored. > > Err? Is this really true, I know about some incompatible changes > to the device tree which prevents you from using usb (or even a > kernel panic) with the imx8mm and I know that on the ls1028a > flexspi wont work if the devicetree doesn't match the kernel. > I.e. if you have a device tree from kernel 5.14 you cannot > use it on 5.10. If you have one from 5.10 you cannot use it on > a later kernel. > >>> > >>> What you describe is the situation we want to avoid and that comes > >>> from years of tinkering. > >> > >> how is that tinkering? That means once you have a device tree, it is > >> set in stone. which isn't reality, sorry. > >> > >> Consider the ls1028a and the flexspi "bug". For the 5.10 kernel/dtb > >> there was a wrong clock connected to the flexspi device. There > >> wasn't > >> even a driver for the correct clock. > > > > The clock could have been described even though there was no Linux > > driver. > > DT is there to describe HW, not the capacity of a particular OS or > > boot firmware to deal with it. > > You're missing my point. It's not about what would be the perfect > scenario. But what actually happens in reality. And unfortunately, > it happens, so you have to deal with devicetree incompatibilies. > Just ignoring this and keeping just one devicetree in the firmware > is doomed to fail sooner or later. We have an example of firmware provided hardware description that works well (Ok: 99% of the time): ACPI. We also are 100% sure that the current situation is messy, hairy, buggy but familiar. > > > [..] > > >> Regarding the imx8mm usb error, apparently the node was renamed. If > >> you're > >> using older kernels with newer dtbs, the kernel oopes. If you're > >> using a > >> newer kernel with older dtbs, USB doesn't get probed. (Although I > >> was > >> told that there is a "fix" for this, that is, both node names are > >> tried.) > > > > I keep seeing those issues about node renames or compatible string > > changes. > > That's the tinkering I am talking about. > > There is no in-kernel ABI but Linus bang heads if anyone touches > > userland-kernel ABI inappropriately. > > > > As DT is mostly handled in-kernel, people treat DT as no-ABI. > > But it is part of firmware-kernel ABI. > > Until we properly organize this firmware-kernel ABI, the problem you > > describe and more will continue. > > Sure, but until you reaches that point (I predict it wont happen soon > or at all) you'll have to deal with per kernel devicetrees. Just > saying, we are just providing one devicetree supplied by the firmware > just doesn't work with the current situation. So IMHO if SystemReady is > really "it just works", then you have to consider this. And so far, > it seems all SystemReady certified boards just throw the u-boot > devicetree at linux and hope for the best. SystemReady is not meant to be all boards, starting now and mandatory. It is a selected of boards for which everyone in the value chain is willing to spend the energy to solve the problem as if we were living in a perfect world. > > > > Now the DT lifecycle before the firmware-kernel ABI also needs to be > > specified so that we properly handle hats, capes, SoMs, carrier > > boards... > > > >>> > >> > > > https://developer.arm.com/architectures/system-architectures/arm-systemready/ir > >>> lists a number of certified boards and the list is going to grow > >>> significantly. > >> > >> And the sl28 board will likely be there soon, too. > >> > >>> On those boards, you will be able to boot any kernel. > >> > >> Actually I don't think so, because you might hit the imx8mm bug, > >> too. > >> May > >> just be lucky by the devicetree/kernel combination. > >> > >>> The image I have in mind with OS provided DT is: > >>> as a French driver, my DT says that the steering wheel is on the > >> left > >>> hand side of the car. > >>> Shall I whine about the cars in England that do not comply to my > >> DT? > >>> or accept to use the car provided DT? > >>> The situation is comfortable for tinkerers, but not sustainable. > >> It is > >>> a matter of organization and not technology to solve the problems > >> you > >>> describe. > >> > >> Mh, I'm not sure I understand what you're trying to tell me. The > >> distribution also provides you the kernel, why shouldn't it provide > >> devicetree which exactly matches this kernel and was also tested > >> against. > > > > Because the kernel has no clue which hats, capes has been added for > > instance. > > And the same might be true for the firmware as pointed out before. > > > The kernel provided DTB works fine when the firmware builder and OS > > builders are the same. > > Ehh? It is just the other way around. The distro supplies the kernel > and thus it also have the corresponding devicetrees for this particular > kernel. The firmware can remain the same. Now Mark mig
Re: Driver model at UEFI runtime
With SystemReady, DT from distros are ignored. Err? Is this really true, I know about some incompatible changes to the device tree which prevents you from using usb (or even a kernel panic) with the imx8mm and I know that on the ls1028a flexspi wont work if the devicetree doesn't match the kernel. I.e. if you have a device tree from kernel 5.14 you cannot use it on 5.10. If you have one from 5.10 you cannot use it on a later kernel. What you describe is the situation we want to avoid and that comes from years of tinkering. how is that tinkering? That means once you have a device tree, it is set in stone. which isn't reality, sorry. Consider the ls1028a and the flexspi "bug". For the 5.10 kernel/dtb there was a wrong clock connected to the flexspi device. There wasn't even a driver for the correct clock. The clock could have been described even though there was no Linux driver. DT is there to describe HW, not the capacity of a particular OS or boot firmware to deal with it. You're missing my point. It's not about what would be the perfect scenario. But what actually happens in reality. And unfortunately, it happens, so you have to deal with devicetree incompatibilies. Just ignoring this and keeping just one devicetree in the firmware is doomed to fail sooner or later. [..] Regarding the imx8mm usb error, apparently the node was renamed. If you're using older kernels with newer dtbs, the kernel oopes. If you're using a newer kernel with older dtbs, USB doesn't get probed. (Although I was told that there is a "fix" for this, that is, both node names are tried.) I keep seeing those issues about node renames or compatible string changes. That's the tinkering I am talking about. There is no in-kernel ABI but Linus bang heads if anyone touches userland-kernel ABI inappropriately. As DT is mostly handled in-kernel, people treat DT as no-ABI. But it is part of firmware-kernel ABI. Until we properly organize this firmware-kernel ABI, the problem you describe and more will continue. Sure, but until you reaches that point (I predict it wont happen soon or at all) you'll have to deal with per kernel devicetrees. Just saying, we are just providing one devicetree supplied by the firmware just doesn't work with the current situation. So IMHO if SystemReady is really "it just works", then you have to consider this. And so far, it seems all SystemReady certified boards just throw the u-boot devicetree at linux and hope for the best. Now the DT lifecycle before the firmware-kernel ABI also needs to be specified so that we properly handle hats, capes, SoMs, carrier boards... https://developer.arm.com/architectures/system-architectures/arm-systemready/ir lists a number of certified boards and the list is going to grow significantly. And the sl28 board will likely be there soon, too. On those boards, you will be able to boot any kernel. Actually I don't think so, because you might hit the imx8mm bug, too. May just be lucky by the devicetree/kernel combination. The image I have in mind with OS provided DT is: as a French driver, my DT says that the steering wheel is on the left hand side of the car. Shall I whine about the cars in England that do not comply to my DT? or accept to use the car provided DT? The situation is comfortable for tinkerers, but not sustainable. It is a matter of organization and not technology to solve the problems you describe. Mh, I'm not sure I understand what you're trying to tell me. The distribution also provides you the kernel, why shouldn't it provide devicetree which exactly matches this kernel and was also tested against. Because the kernel has no clue which hats, capes has been added for instance. And the same might be true for the firmware as pointed out before. The kernel provided DTB works fine when the firmware builder and OS builders are the same. Ehh? It is just the other way around. The distro supplies the kernel and thus it also have the corresponding devicetrees for this particular kernel. The firmware can remain the same. Now Mark might disagree here, because OpenBSD doesn't provide devicetrees (I really don't know). I agree, that in a perfect world, there would be just one (or a set of) stable devicetree(s) which can be used by any OS/Distribution/Kernel. But this simply isn't the case. This traditional model is evolving and the team that deals with OS may not even be in the same company as the one providing the firmware. Ask the Fedora IoT architect what he thinks about the distro provided DTs ;-) I don't even know who "the fedora iot architect" is, nor what her/his arguments are. There is a need to deal with DT bugs. OS provided DT is a bad solution to a real problem. U-Boot patches look a possibility for those bugs. And then you always have to update the firmware and duplicate the "code". I.e. theres an incompatible change, the devicetree is update in linux and you have to replicate just this as a fixup in u-boot. A
Re: Status of the various RISC-V specification and policy
On Thu, 30 Sep 2021 10:38:02 PDT (-0700), markhimelst...@riscv.org wrote: The following is the extension lifecycle. It includes the official names going forward for each phase. We are trying to resolve any confusion naming and numbering and are still in progress of this evolution: https://docs.google.com/presentation/d/1nQ5uFb39KA6gvUi5SReWfIQSiRN7hp6z7ZPfctE4mKk/edit?usp=sharing Again, if we can improve anything to make it clearer or if we got something wrong, please let us know. That is, unfortunately, even more confusing. Slide 3 lists the milestones, but that uses very different terms than the "Specification States" wiki entry I was linked to earlier as the canonical definition of the process. I'm also now less sure about what exactly is being frozen, as the slides seem to mix up extension and specification (which is the core of what I'm worried about). Looking at slide 4 (titled "Extension Lifecycle"), I see a bunch of version number looking strings (things like "v0.1" and "v1.0-rcN (final)"). Are those versions, and if so what do they version? It also says "v1.0 (ratified)" with an arrow pointing directly after "TSC Ratification Review & Vote", but in the v-1.0 tag I see "Once ratified, the spec will be given version 2.0." Are these version-number-looking strings supposed to be things that exist within the same namespace? Just loking over the slide again I see "New or Changed Features Specification Development become a new extension -- Go back to the top left". That sort of seems like something that might help answer some of my core questionsn here around what's allowed to change when, but I'm genuinely not sure how to parse the words. Might not be the most important thing to focus on now, though. Mark On Thu, Sep 30, 2021 at 10:30 AM Palmer Dabbelt wrote: On Thu, 30 Sep 2021 08:06:42 PDT (-0700), markhimelst...@riscv.org wrote: > Palmer, > > > > Thank you for your input. > > > > Our strong intention is to not change specs once frozen. I speak for the > committees here and say that, in our opinion, declaring something frozen > sets a very high bar for making any changes and is sufficient to allow code > supporting an extension to be upstreamed. Of course if an unexpected and > significant issue is discovered during the public review that absolutely > must be addressed and cannot be deferred to a future extension (where the > cost of not addressing the issue exceeds the cost of addressing it. for > example introduces security vulnerabilities), then we will do so, as anyone > should expect from a public review. > > > > We do not have versions of extensions. If an extension has a problem once > ratified, we will issue errata. All implementers have to publish the errata > if they use branding. We may release a new extension with the bulk of the > original extension plus the errata fix at some future date. This is probably at the core of my confusion here. At the preface of the user ISA there is a table with the headings "Extension", "Version", and "Frozen"; contains a list of letters that look like extension name; and contains a list of numbers that look like versions of those extensions. That nomenclature seems to carry on to some more recent specifications. For example the first page of https://github.com/riscv/riscv-v-spec/releases/download/v1.0/riscv-v-spec-1.0.pdf (tagged 11 days ago) is RISC-V "V" Vector Extension Version 1.0 I'm happy to answer the rest of the questions here, but I think trying to get on the same page about what is versioned and is proabbly the first step because that's a pretty key component of my worries. > New extensions reserve the right to be incompatible with existing > extensions but our philosophy is very much to minimize that and only allow > the rare well-justified exceptions. Reasons may include errata, security > issues discovered, or new functionality we need to add that justifies > creating an incompatibility, etc. > > What specifically do you see as an issue? What are you blocked on by our > conventions? We need specific details to resolve any issues. Right now, I > don't feel I have enough information from you. > > > > Thanks > > Mark > > > > P.S. We had some situations in the past, in part due to vendors not waiting > for the specification processes to conclude, where implementers implemented > non-confoming chips either with vendor-specific extensions using reserved > opcodes and state, or implementing early drafts of standards-track > proposals in the development state (will likely change). This is in the > past and resolved. Anyone implementing non-standard extensions must > advertise them as such and make it clear that these are not standard RISC-V > extensions: this should make it clear for upstream projects that they will > be dealing with the respective vendors for support and maintenance, and > that any code implementing support for these extensions will be different > from what covers the respective
Re: [PATCH] cmd: sf: Support unaligned flash updates with 'sf update'
Am 2021-09-30 18:19, schrieb Frieder Schrempf:
From: Frieder Schrempf
Currently 'sf update' supports only offsets that are aligned to the
erase block size of the serial flash. Unaligned offsets result in
something like:
=> sf update ${kernel_addr_r} 0x400 ${filesize}
device 0 offset 0x400, size 0x11f758
SPI flash failed in erase step
In order to support unaligned updates, we simply read the first full
block and check only the requested part of the block for changes. If
necessary, the block is erased, the first (unchanged) part of the block
is written back together with the second part of the block (updated
data).
Signed-off-by: Frieder Schrempf
---
cmd/sf.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c
index eac27ed2d7..c54b4b10bb 100644
--- a/cmd/sf.c
+++ b/cmd/sf.c
@@ -171,30 +171,32 @@ static int do_spi_flash_probe(int argc, char
*const argv[])
static const char *spi_flash_update_block(struct spi_flash *flash, u32
offset,
size_t len, const char *buf, char *cmp_buf, size_t *skipped)
{
+ u32 offset_aligned = ALIGN_DOWN(offset, flash->sector_size);
+ u32 sector_offset = offset - offset_aligned;
char *ptr = (char *)buf;
debug("offset=%#x, sector_size=%#x, len=%#zx\n",
offset, flash->sector_size, len);
/* Read the entire sector so to allow for rewriting */
- if (spi_flash_read(flash, offset, flash->sector_size, cmp_buf))
+ if (spi_flash_read(flash, offset_aligned, flash->sector_size,
cmp_buf))
return "read";
/* Compare only what is meaningful (len) */
- if (memcmp(cmp_buf, buf, len) == 0) {
+ if (memcmp(cmp_buf + sector_offset, buf, len) == 0) {
debug("Skip region %x size %zx: no change\n",
offset, len);
*skipped += len;
return NULL;
}
/* Erase the entire sector */
- if (spi_flash_erase(flash, offset, flash->sector_size))
+ if (spi_flash_erase(flash, offset_aligned, flash->sector_size))
return "erase";
/* If it's a partial sector, copy the data into the temp-buffer */
if (len != flash->sector_size) {
- memcpy(cmp_buf, buf, len);
+ memcpy(cmp_buf + sector_offset, buf, len);
ptr = cmp_buf;
}
/* Write one complete sector */
- if (spi_flash_write(flash, offset, flash->sector_size, ptr))
+ if (spi_flash_write(flash, offset_aligned, flash->sector_size, ptr))
return "write";
return NULL;
@@ -230,7 +232,10 @@ static int spi_flash_update(struct spi_flash
*flash, u32 offset,
ulong last_update = get_timer(0);
for (; buf < end && !err_oper; buf += todo, offset += todo) {
- todo = min_t(size_t, end - buf, flash->sector_size);
+ if (offset % flash->sector_size)
do_div() to avoid linking errors on some archs, I guess.
+ todo = flash->sector_size - (offset %
flash->sector_size);
This is missing the end-buf calculation, no? I.e. if you update just one
sector at an offset and the data you write is smaller than "sector_size
- offset".
+ else
+ todo = min_t(size_t, end - buf,
flash->sector_size);
if (get_timer(last_update) > 100) {
printf(" \rUpdating, %zu%% %lu B/s",
100 - (end - buf) / scale,
-michael
Re: [PATCH] cmd: sf: Support unaligned flash updates with 'sf update'
Am 2021-09-30 19:17, schrieb Frieder Schrempf: On 30.09.21 18:35, Michael Walle wrote: Am 2021-09-30 18:19, schrieb Frieder Schrempf: In order to support unaligned updates, we simply read the first full block and check only the requested part of the block for changes. If necessary, the block is erased, the first (unchanged) part of the block is written back together with the second part of the block (updated data). I'm not sure what I should think of this. You might loose that unchanged part if there is an power loss in the middle, even worse, you likely don't have this part anymore because it isn't part of the data you want to write to the flash. Maybe add an parameter for allow (unsafe) unaligned updates? Now you might argue, that with "sf erase, sf write" you can do the same harm, to which i reply: but then it is intentional ;) Because "sf erase" just works on sector boundaries (which isn't really checked in the command, i just realized, but at least my driver returns EINVAL) and then the user has to include the "unchanged part" for the arguments on the commandline. True, but "sf update" already is "unsafe" even without supporting unaligned start offsets. The code already handles partial sector writes for the last sector in the same way (read, erase, write), which is also prone to data loss in case of power loss. Ah, I missed that. Yes, in this case, we don't loose anything. Agreed. So this patch in fact just adds support for partial sector updates for the first sector. It is slightly more probable to loose data, but it doesn't introduce new "unsafe" behavior. Maybe we could cover this by adding a warning to the documentation, or even printing a warning? Heh, although I was using "sf update" all the time, I wasn't aware of the read - "partly modify" - write cycle. Maybe it's just me being over cautious. Printing a warning might scare users, though. I'd prefer to fix the online help, where only "erase and write" is mentioned. -michael
Re: CI and ca-certificates packages
On Thu, Sep 30, 2021 at 11:34:11AM -0400, Tom Rini wrote: > Hey all, > > Due to the combination of the Let's Encrypt root certificate change and > the CI images not ending up with a new enough ca-certificates package, > CI is currently failing. It looks like the best fix for this, sadly, is > to rebuild the CI images, so I'll be doing that, uploading to Docker and > pushing those changes as soon as I can. I've now respun both images and pushed a change for master, and next, to use these new images. -- Tom signature.asc Description: PGP signature
Re: [GIT PULL] xilinx patches for v2022.01-rc1
On Thu, Sep 30, 2021 at 04:50:11PM +0200, Michal Simek wrote: > Hi Tom, > > I want to clean my queue for next version that's why please merge these > patches to your tree. > > Thanks, > Michal > > > The following changes since commit 67ae2897235e516d8fa9ab3f296a1caf40f6ebee: > > Merge tag 'rpi-next-2021.10.2' of > https://source.denx.de/u-boot/custodians/u-boot-raspberrypi (2021-09-29 > 15:13:35 -0400) > > are available in the Git repository at: > > g...@source.denx.de:u-boot/custodians/u-boot-microblaze.git > tags/xilinx-for-v2022.01-rc1 > > for you to fetch changes up to dced079c53b283e15f04282f405de410a9be584d: > > watchdog: versal: Add support for expire now (2021-09-30 12:30:38 +0200) > Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH u-boot-marvell v3 00/39] kwboot higher baudrate
Hello! Could you test or review this patch series? It is a big improvement for kwboot as it allows to transfer u-boot over uart into mvebu platforms much faster. On Friday 24 September 2021 23:06:37 Marek Behún wrote: > From: Marek Behún > > Hello Stefan and others, > > here's v3 of series adding support for booting Marvell platforms via > UART (those bootable with kwboot) at higher baudrates. > > Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than > 115200 Bd. > > The user can direct kwboot to use higher baudrate via the -B option. > (BTW this option was there before but did not work with the -b option.) > > Only the payload part of the KWB image is uploaded at this higher > baudrate. The header part is still uploaded at 115200 Bd, since the code > that changes baudrate is injected into header as a binary extension. > (The payload part makes up majority of the binary, though. On Turris > Omnia the payload currently makes ~87%.) > > The series also contains various other fixes, refactors and improvements > of the code, upon which the main change is done. > > Marek & Pali > > Changes since v2: > - fixed integer overflow in patch 15 > - fixed commit title in patch 32 > > Changes since v1: > - fixed uploading of older kwb images, now all kwb images should be able > to upload at faster baudrate > - fixed injecting code that changes baudrate back > - various other fixes and refactors, the best way to compare with v1 is > to apply v1 and v2 separately and compare the resulting kwboot.c > > Marek Behún (19): > tools: kwbimage: Fix printf format warning > tools: kwboot: Fix buffer overflow in kwboot_terminal() > tools: kwboot: Make the quit sequence buffer const > tools: kwboot: Refactor and fix writing buffer > tools: kwboot: Fix comparison of integers with different size > tools: kwboot: Use a function to check whether received byte is a > Xmodem reply > tools: kwboot: Print new line after SPL output > tools: kwboot: Allow greater timeout when executing header code > tools: kwboot: Prevent waiting indefinitely if no xmodem reply is > received > tools: kwbimage: Simplify iteration over version 1 optional headers > tools: kwbimage: Refactor image_version() > tools: kwbimage: Refactor kwbimage header size determination > tools: kwboot: Explicitly check against size of struct main_hdr_v1 > tools: kwboot: Check whether baudrate was set to requested value > tools: kwboot: Cosmetic fix > tools: kwboot: Avoid code repetition in kwboot_img_patch() > tools: kwboot: Update file header > doc/kwboot.1: Update man page > MAINTAINERS: Add entry for kwbimage / kwboot tools > > Pali Rohár (20): > tools: kwboot: Print version information header > tools: kwboot: Fix kwboot_xm_sendblock() function when > kwboot_tty_recv() fails > tools: kwboot: Fix return type of kwboot_xm_makeblock() function > tools: kwboot: Fix printing progress > tools: kwboot: Print newline on error when progress was not completed > tools: kwboot: Split sending image into header and data stages > tools: kwboot: Allow non-xmodem text output from BootROM only in a > specific case > tools: kwboot: Properly finish xmodem transfer > tools: kwboot: Always call kwboot_img_patch_hdr() > tools: kwboot: Don't patch image header if signed > tools: kwboot: Patch source address in image header > tools: kwboot: Patch destination address to DDR area for SPI image > tools: kwbimage: Update comments describing kwbimage v1 structures > tools: kwboot: Round up header size to 128 B when patching > tools: kwboot: Support higher baudrates when booting via UART > tools: kwboot: Allow any baudrate on Linux > tools: kwboot: Fix initializing tty device > tools: kwboot: Disable tty interbyte timeout > tools: kwboot: Disable non-blocking mode > tools: kwboot: Add Pali and Marek as authors > > MAINTAINERS | 10 + > doc/kwboot.1 | 60 ++- > tools/kwbimage.c | 130 ++--- > tools/kwbimage.h | 99 +++- > tools/kwboot.c| 1197 +++-- > tools/termios_linux.h | 189 +++ > 6 files changed, 1385 insertions(+), 300 deletions(-) > create mode 100644 tools/termios_linux.h > > -- > 2.32.0 >
Re: [PATCH] acpi: Use U-Boot version for OEM_REVISION
On Wed, Sep 29, 2021 at 10:08:58PM -0600, Simon Glass wrote: > Hi Pali, > > On Sun, 12 Sept 2021 at 15:30, Pali Rohár wrote: > > > > On Tuesday 20 July 2021 12:32:46 Simon Glass wrote: > > > On Sat, 10 Jul 2021 at 05:10, Pali Rohár wrote: > > > > > > > > OEM_REVISION is 32-bit unsigned number. It should be increased only when > > > > changing software version. Therefore it should not depend on build time. > > > > > > > > Change calculation to use U-Boot version numbers and set this revision > > > > to date number. > > > > > > > > Prior this change OEM_REVISION was calculated from build date and > > > > stored in > > > > the same format. > > > > > > > > After this change macro U_BOOT_BUILD_DATE is not used in other files so > > > > remove it from global autogenerated files and also from Makefile. > > > > > > > > Signed-off-by: Pali Rohár > > > > --- > > > > This patch depends on similar patch for BIOS Release Date which is here: > > > > http://patchwork.ozlabs.org/project/uboot/patch/20210422160957.26936-1-p...@kernel.org/ > > > > --- > > > > Makefile| 2 -- > > > > doc/develop/version.rst | 1 - > > > > lib/acpi/acpi_table.c | 18 +- > > > > test/dm/acpi.c | 20 ++-- > > > > 4 files changed, 31 insertions(+), 10 deletions(-) > > > > > > Reviewed-by: Simon Glass > > > > Hello! Could you process this patch? Or are there any issues? > > +Tom Rini > > It isn't in my queue. Perhaps Tom has it? Well, hunh. I don't know when I moved it to Accepted, but it clearly wasn't. Sorry about that. -- Tom signature.asc Description: PGP signature
Re: Status of the various RISC-V specification and policy
The following is the extension lifecycle. It includes the official names going forward for each phase. We are trying to resolve any confusion naming and numbering and are still in progress of this evolution: https://docs.google.com/presentation/d/1nQ5uFb39KA6gvUi5SReWfIQSiRN7hp6z7ZPfctE4mKk/edit?usp=sharing Again, if we can improve anything to make it clearer or if we got something wrong, please let us know. Mark On Thu, Sep 30, 2021 at 10:30 AM Palmer Dabbelt wrote: > On Thu, 30 Sep 2021 08:06:42 PDT (-0700), markhimelst...@riscv.org wrote: > > Palmer, > > > > > > > > Thank you for your input. > > > > > > > > Our strong intention is to not change specs once frozen. I speak for the > > committees here and say that, in our opinion, declaring something frozen > > sets a very high bar for making any changes and is sufficient to allow > code > > supporting an extension to be upstreamed. Of course if an unexpected and > > significant issue is discovered during the public review that absolutely > > must be addressed and cannot be deferred to a future extension (where the > > cost of not addressing the issue exceeds the cost of addressing it. for > > example introduces security vulnerabilities), then we will do so, as > anyone > > should expect from a public review. > > > > > > > > We do not have versions of extensions. If an extension has a problem once > > ratified, we will issue errata. All implementers have to publish the > errata > > if they use branding. We may release a new extension with the bulk of the > > original extension plus the errata fix at some future date. > > This is probably at the core of my confusion here. > > At the preface of the user ISA there is a table with the headings > "Extension", "Version", and "Frozen"; contains a list of letters that > look like extension name; and contains a list of numbers that look like > versions of those extensions. > > That nomenclature seems to carry on to some more recent specifications. > For example the first page of > > https://github.com/riscv/riscv-v-spec/releases/download/v1.0/riscv-v-spec-1.0.pdf > (tagged 11 days ago) is > > RISC-V "V" Vector Extension > Version 1.0 > > I'm happy to answer the rest of the questions here, but I think trying > to get on the same page about what is versioned and is proabbly the > first step because that's a pretty key component of my worries. > > > New extensions reserve the right to be incompatible with existing > > extensions but our philosophy is very much to minimize that and only > allow > > the rare well-justified exceptions. Reasons may include errata, security > > issues discovered, or new functionality we need to add that justifies > > creating an incompatibility, etc. > > > > What specifically do you see as an issue? What are you blocked on by our > > conventions? We need specific details to resolve any issues. Right now, I > > don't feel I have enough information from you. > > > > > > > > Thanks > > > > Mark > > > > > > > > P.S. We had some situations in the past, in part due to vendors not > waiting > > for the specification processes to conclude, where implementers > implemented > > non-confoming chips either with vendor-specific extensions using reserved > > opcodes and state, or implementing early drafts of standards-track > > proposals in the development state (will likely change). This is in the > > past and resolved. Anyone implementing non-standard extensions must > > advertise them as such and make it clear that these are not standard > RISC-V > > extensions: this should make it clear for upstream projects that they > will > > be dealing with the respective vendors for support and maintenance, and > > that any code implementing support for these extensions will be different > > from what covers the respective standard extensions. Whether upstream > > projects accept such changes, and what conditions they stipulate for > > acceptance of these changes, are beyond the control of RISC-V. We also, > as > > I have described to you many times, have instituted mandatory standards > > specification states for the front page of each specification to ensure > > clarity (any divergence from this is a bug and we work to fix these > > quickly). > > > > > > On Tue, Sep 28, 2021 at 11:34 AM Palmer Dabbelt > wrote: > > > >> On Mon, 27 Sep 2021 08:57:15 PDT (-0700), markhimelst...@riscv.org > wrote: > >> > the words in this document : > >> > > >> > > >> > https://wiki.riscv.org/plugins/servlet/mobile?contentId=13098230#content/view/13098230 > >> > > >> > make it very clear when changes are allowed or not and likely or not. > >> > > >> > if you think the verbiage is somehow ambiguous please help us make it > >> better. > >> > >> I'm not really worried about changes, I'm worried about a committment to > >> future compatibility. When we take code into the kernel (and most other > >> core systems projects) we're taking on the burden of supporting (until > >> someone can prove there are no more users),
Re: Status of the various RISC-V specification and policy
On Thu, 30 Sep 2021 08:06:42 PDT (-0700), markhimelst...@riscv.org wrote: Palmer, Thank you for your input. Our strong intention is to not change specs once frozen. I speak for the committees here and say that, in our opinion, declaring something frozen sets a very high bar for making any changes and is sufficient to allow code supporting an extension to be upstreamed. Of course if an unexpected and significant issue is discovered during the public review that absolutely must be addressed and cannot be deferred to a future extension (where the cost of not addressing the issue exceeds the cost of addressing it. for example introduces security vulnerabilities), then we will do so, as anyone should expect from a public review. We do not have versions of extensions. If an extension has a problem once ratified, we will issue errata. All implementers have to publish the errata if they use branding. We may release a new extension with the bulk of the original extension plus the errata fix at some future date. This is probably at the core of my confusion here. At the preface of the user ISA there is a table with the headings "Extension", "Version", and "Frozen"; contains a list of letters that look like extension name; and contains a list of numbers that look like versions of those extensions. That nomenclature seems to carry on to some more recent specifications. For example the first page of https://github.com/riscv/riscv-v-spec/releases/download/v1.0/riscv-v-spec-1.0.pdf (tagged 11 days ago) is RISC-V "V" Vector Extension Version 1.0 I'm happy to answer the rest of the questions here, but I think trying to get on the same page about what is versioned and is proabbly the first step because that's a pretty key component of my worries. New extensions reserve the right to be incompatible with existing extensions but our philosophy is very much to minimize that and only allow the rare well-justified exceptions. Reasons may include errata, security issues discovered, or new functionality we need to add that justifies creating an incompatibility, etc. What specifically do you see as an issue? What are you blocked on by our conventions? We need specific details to resolve any issues. Right now, I don't feel I have enough information from you. Thanks Mark P.S. We had some situations in the past, in part due to vendors not waiting for the specification processes to conclude, where implementers implemented non-confoming chips either with vendor-specific extensions using reserved opcodes and state, or implementing early drafts of standards-track proposals in the development state (will likely change). This is in the past and resolved. Anyone implementing non-standard extensions must advertise them as such and make it clear that these are not standard RISC-V extensions: this should make it clear for upstream projects that they will be dealing with the respective vendors for support and maintenance, and that any code implementing support for these extensions will be different from what covers the respective standard extensions. Whether upstream projects accept such changes, and what conditions they stipulate for acceptance of these changes, are beyond the control of RISC-V. We also, as I have described to you many times, have instituted mandatory standards specification states for the front page of each specification to ensure clarity (any divergence from this is a bug and we work to fix these quickly). On Tue, Sep 28, 2021 at 11:34 AM Palmer Dabbelt wrote: On Mon, 27 Sep 2021 08:57:15 PDT (-0700), markhimelst...@riscv.org wrote: > the words in this document : > > https://wiki.riscv.org/plugins/servlet/mobile?contentId=13098230#content/view/13098230 > > make it very clear when changes are allowed or not and likely or not. > > if you think the verbiage is somehow ambiguous please help us make it better. I'm not really worried about changes, I'm worried about a committment to future compatibility. When we take code into the kernel (and most other core systems projects) we're taking on the burden of supporting (until someone can prove there are no more users), which is very difficult to do when the ISA changes in an incompatible fashion. The whole point of agreeing on the frozen thing was that it gave us a committment from the specifcation authors that the future ISA would be compatible with th frozen extensions. We're already in this spot with the V extension and the whole stable thing, this definitaion of frozen looks very much like what was has led to the issues there. Saying the spec won't change really isn't meaningful, it's saying future specs will be compatible that's important. Nothing in this whole rule touches on compatibility, and I really don't want to end up in a bigger mess than we're already in. (Also: some PGE subcontractor drove a crane into my house, so things are a bit chaotic on my end. If you have that list of what's officially frozen, can you sen
Re: [PATCH] cmd: sf: Support unaligned flash updates with 'sf update'
On 30.09.21 18:35, Michael Walle wrote: > Am 2021-09-30 18:19, schrieb Frieder Schrempf: >> In order to support unaligned updates, we simply read the first full >> block and check only the requested part of the block for changes. If >> necessary, the block is erased, the first (unchanged) part of the block >> is written back together with the second part of the block (updated >> data). > > I'm not sure what I should think of this. You might loose that unchanged > part if there is an power loss in the middle, even worse, you likely don't > have this part anymore because it isn't part of the data you want to write > to the flash. > > Maybe add an parameter for allow (unsafe) unaligned updates? > > Now you might argue, that with "sf erase, sf write" you can do the same > harm, to which i reply: but then it is intentional ;) Because "sf erase" > just works on sector boundaries (which isn't really checked in the command, > i just realized, but at least my driver returns EINVAL) and then the > user has to include the "unchanged part" for the arguments on the > commandline. True, but "sf update" already is "unsafe" even without supporting unaligned start offsets. The code already handles partial sector writes for the last sector in the same way (read, erase, write), which is also prone to data loss in case of power loss. So this patch in fact just adds support for partial sector updates for the first sector. It is slightly more probable to loose data, but it doesn't introduce new "unsafe" behavior. Maybe we could cover this by adding a warning to the documentation, or even printing a warning?
Re: Driver model at UEFI runtime
On Thu, 30 Sept 2021 at 18:25, Michael Walle wrote: > Am 2021-09-30 17:47, schrieb François Ozog: > > On Thu, 30 Sept 2021 at 17:12, Michael Walle wrote: > > > >> [adding Vladimir, because he showed interest in this, too] > >> > >> Am 2021-09-30 15:56, schrieb François Ozog: > >>> On Thu, 30 Sept 2021 at 14:07, Michael Walle > >> wrote: > >>> > Am 2021-09-30 12:50, schrieb Heinrich Schuchardt: > > Am 30. September 2021 11:53:47 MESZ schrieb Michael Walle > > : > >> Am 2021-09-30 08:56, schrieb Heinrich Schuchardt: > >>> On 9/30/21 8:23 AM, François Ozog wrote: > >> [..] > Is U-Boot's UEFI loader implementation supposed to be the > recommended > UEFI firmware on ARM and RISC-V on a production / > deployment > system? > > For Arm: yes, that is SystemReady spec. > >>> > >>> For RISC-V it is required by the RISC-V platform > >> specification. > >>> > > > Do we expect bootefi to boot a kernel with CONFIG_EFI_STUB, > or > do > we > expect to load grub.efi which chain-loads a kernel without > CONFIG_EFI_STUB? > > all paths should be possible , and the shim.efi is to be > supported > too. > With UEFI, I always see that UEFI is kept down to OS for > SecureBoot. > In > other words I don’t see grub.efi booting a non > config_efi_stub. > > What do distributions normally do? > > At least Red Hat made it clear at multiple Linaro Connect > >> that > they > want > standards, and SystemReady is one that makes the life of > embedded > distros easy. > Distros boot shim.efi, grub.efi, Linux.efi to benefit from > >> UEFi > SecureBoot. > >>> > >>> For Fedora see > >>> > > >>> > >> > > https://fedoraproject.org/wiki/Changes/uEFIforARMv7#Detailed_Description > >>> > >>> SUSE started the UEFI implementation to boot on all > architectures in > >>> the > >>> same way. The current ARM and RISC-V images uses UEFI. > >>> > >>> Debian and Ubuntu install for booting via GRUB if the > >> installer > is > >>> invoked via UEFI. A fallback solution using the legacy Linux > entry > >>> point > >>> exists. > >>> > >>> For BSD the only way to boot on ARM is via UEFI. > >>> > > What's our > position when compared to EDK II? > >>> > >>> U-Boot implements only a subset of UEFI defined in the EBBR > >>> specification. > >>> > >>> For servers you need a larger scope which is offered by EDK > >> II. > This > >>> is > >>> required both by the RISC-V platform specification as well as > the ARM > >>> SystemReady ServerReady profile. > >>> > >>> The number of boards supported by upstream EDK II is very low. > But > >>> proprietary firmware based on EDK II exists. > >>> > > the typical distro boot flow is not the most efficient and > drags > concept > dating where the Microsoft certs had to be part of the > >> picture. > A > direct > U-Boot Linux.efi is my preferred; avoids yet another OS in > >> the > boot > path > (grub), avoids convoluted platform cert management (shim) and > just > >>> > >>> This is why U-Boot supports UEFI boot options specifying both > >> a > >>> binary > >>> as well as an initial RAM disk. > >> > >> I might be late to this, but where does the devicetree come > >> from? > As > >> far > >> as I understand it right now, for most (all) the SytemReady IR > >> certified > >> boards, the compiled-in one from u-boot is passed to linux. > >> This > won't > >> work > >> in the long run, because the devicetrees keep getting > incompatible > >> changes. > >> So while it work for one kernel version it might not work on > >> the > next > >> version. > > > > It would make sense to add the fdt devicepath to the bootoption > like > > we did it for the initrd. > > I haven't followed the much of the recent development, are there > >> any > commits/files I can look at? > > And I'm not just talking about the use case where the EFI stub of > linux > is booted directy, but also when there is grub in between. > > The distribution would supply a bunch of devicetrees along with > the kernel (and initrd). Possibly also different versions of > >> these. > In grub you would choose the desired kernel/initrd/devicetree > combination. > >>> > >>> With SystemReady, DT from distros are ignored. > >> > >> Err? Is this really true, I know about some incompatible changes > >> to the device tree which prevents you from using usb (or eve
Re: [PATCH] cmd: sf: Support unaligned flash updates with 'sf update'
Am 2021-09-30 18:19, schrieb Frieder Schrempf: In order to support unaligned updates, we simply read the first full block and check only the requested part of the block for changes. If necessary, the block is erased, the first (unchanged) part of the block is written back together with the second part of the block (updated data). I'm not sure what I should think of this. You might loose that unchanged part if there is an power loss in the middle, even worse, you likely don't have this part anymore because it isn't part of the data you want to write to the flash. Maybe add an parameter for allow (unsafe) unaligned updates? Now you might argue, that with "sf erase, sf write" you can do the same harm, to which i reply: but then it is intentional ;) Because "sf erase" just works on sector boundaries (which isn't really checked in the command, i just realized, but at least my driver returns EINVAL) and then the user has to include the "unchanged part" for the arguments on the commandline. -michael
Re: Driver model at UEFI runtime
Am 2021-09-30 17:47, schrieb François Ozog: On Thu, 30 Sept 2021 at 17:12, Michael Walle wrote: [adding Vladimir, because he showed interest in this, too] Am 2021-09-30 15:56, schrieb François Ozog: On Thu, 30 Sept 2021 at 14:07, Michael Walle wrote: Am 2021-09-30 12:50, schrieb Heinrich Schuchardt: Am 30. September 2021 11:53:47 MESZ schrieb Michael Walle : Am 2021-09-30 08:56, schrieb Heinrich Schuchardt: On 9/30/21 8:23 AM, François Ozog wrote: [..] Is U-Boot's UEFI loader implementation supposed to be the recommended UEFI firmware on ARM and RISC-V on a production / deployment system? For Arm: yes, that is SystemReady spec. For RISC-V it is required by the RISC-V platform specification. Do we expect bootefi to boot a kernel with CONFIG_EFI_STUB, or do we expect to load grub.efi which chain-loads a kernel without CONFIG_EFI_STUB? all paths should be possible , and the shim.efi is to be supported too. With UEFI, I always see that UEFI is kept down to OS for SecureBoot. In other words I don’t see grub.efi booting a non config_efi_stub. What do distributions normally do? At least Red Hat made it clear at multiple Linaro Connect that they want standards, and SystemReady is one that makes the life of embedded distros easy. Distros boot shim.efi, grub.efi, Linux.efi to benefit from UEFi SecureBoot. For Fedora see https://fedoraproject.org/wiki/Changes/uEFIforARMv7#Detailed_Description SUSE started the UEFI implementation to boot on all architectures in the same way. The current ARM and RISC-V images uses UEFI. Debian and Ubuntu install for booting via GRUB if the installer is invoked via UEFI. A fallback solution using the legacy Linux entry point exists. For BSD the only way to boot on ARM is via UEFI. What's our position when compared to EDK II? U-Boot implements only a subset of UEFI defined in the EBBR specification. For servers you need a larger scope which is offered by EDK II. This is required both by the RISC-V platform specification as well as the ARM SystemReady ServerReady profile. The number of boards supported by upstream EDK II is very low. But proprietary firmware based on EDK II exists. the typical distro boot flow is not the most efficient and drags concept dating where the Microsoft certs had to be part of the picture. A direct U-Boot Linux.efi is my preferred; avoids yet another OS in the boot path (grub), avoids convoluted platform cert management (shim) and just This is why U-Boot supports UEFI boot options specifying both a binary as well as an initial RAM disk. I might be late to this, but where does the devicetree come from? As far as I understand it right now, for most (all) the SytemReady IR certified boards, the compiled-in one from u-boot is passed to linux. This won't work in the long run, because the devicetrees keep getting incompatible changes. So while it work for one kernel version it might not work on the next version. It would make sense to add the fdt devicepath to the bootoption like we did it for the initrd. I haven't followed the much of the recent development, are there any commits/files I can look at? And I'm not just talking about the use case where the EFI stub of linux is booted directy, but also when there is grub in between. The distribution would supply a bunch of devicetrees along with the kernel (and initrd). Possibly also different versions of these. In grub you would choose the desired kernel/initrd/devicetree combination. With SystemReady, DT from distros are ignored. Err? Is this really true, I know about some incompatible changes to the device tree which prevents you from using usb (or even a kernel panic) with the imx8mm and I know that on the ls1028a flexspi wont work if the devicetree doesn't match the kernel. I.e. if you have a device tree from kernel 5.14 you cannot use it on 5.10. If you have one from 5.10 you cannot use it on a later kernel. What you describe is the situation we want to avoid and that comes from years of tinkering. how is that tinkering? That means once you have a device tree, it is set in stone. which isn't reality, sorry. Consider the ls1028a and the flexspi "bug". For the 5.10 kernel/dtb there was a wrong clock connected to the flexspi device. There wasn't even a driver for the correct clock. Thus, I introduced a new clock driver and wired it correctly in the device tree. Which was introduced in 5.11 (or 5.12 I don't know anymore). u-boot is now providing a devicetree from the 5.14 kernel. Thus, it has the clock input connected to the new clock driver. But debian, for example, has kernel 5.10. Which doesn't have the clock driver, oops. The flexspi tries to enable the clock which fails and the whole probe will fail. Regarding the imx8mm usb error, apparently the node was renamed. If you're using older kernels with newer dtbs, the kernel oopes. If you're using a newer kernel with older dtbs, USB doe
