From a13af89e10391189b87ad32616ba688d6d7c5878 Mon Sep 17 00:00:00 2001 From: Maxim Cournoyer Date: Thu, 12 Oct 2023 23:06:24 -0400 Subject: patman: Add a 'keep_change_id' setting A Change-Id can be useful for traceability purposes, and some projects may wish to have them preserved. This change makes it configurable via a new 'keep_change_id' setting. Signed-off-by: Maxim Cournoyer Reviewed-by: Simon Glass --- tools/patman/__main__.py | 2 ++ tools/patman/control.py | 12 +++++++++--- tools/patman/patchstream.py | 17 ++++++++++++----- tools/patman/patman.rst | 11 ++++++----- tools/patman/test_checkpatch.py | 16 ++++++++++++++++ 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/tools/patman/__main__.py b/tools/patman/__main__.py index 8eba5d3..197ac1a 100755 --- a/tools/patman/__main__.py +++ b/tools/patman/__main__.py @@ -103,6 +103,8 @@ send.add_argument('--no-signoff', action='store_false', dest='add_signoff', default=True, help="Don't add Signed-off-by to patches") send.add_argument('--smtp-server', type=str, help="Specify the SMTP server to 'git send-email'") +send.add_argument('--keep-change-id', action='store_true', + help='Preserve Change-Id tags in patches to send.') send.add_argument('patchfiles', nargs='*') diff --git a/tools/patman/control.py b/tools/patman/control.py index 916ddf8..b292da9 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -16,11 +16,14 @@ from patman import gitutil from patman import patchstream from u_boot_pylib import terminal + def setup(): """Do required setup before doing anything""" gitutil.setup() -def prepare_patches(col, branch, count, start, end, ignore_binary, signoff): + +def prepare_patches(col, branch, count, start, end, ignore_binary, signoff, + keep_change_id=False): """Figure out what patches to generate, then generate them The patch files are written to the current directory, e.g. 0001_xxx.patch @@ -35,6 +38,7 @@ def prepare_patches(col, branch, count, start, end, ignore_binary, signoff): end (int): End patch to use (0=last one in series, 1=one before that, etc.) ignore_binary (bool): Don't generate patches for binary files + keep_change_id (bool): Preserve the Change-Id tag. Returns: Tuple: @@ -59,11 +63,12 @@ def prepare_patches(col, branch, count, start, end, ignore_binary, signoff): branch, start, to_do, ignore_binary, series, signoff) # Fix up the patch files to our liking, and insert the cover letter - patchstream.fix_patches(series, patch_files) + patchstream.fix_patches(series, patch_files, keep_change_id) if cover_fname and series.get('cover'): patchstream.insert_cover_letter(cover_fname, series, to_do) return series, cover_fname, patch_files + def check_patches(series, patch_files, run_checkpatch, verbose, use_tree): """Run some checks on a set of patches @@ -166,7 +171,8 @@ def send(args): col = terminal.Color() series, cover_fname, patch_files = prepare_patches( col, args.branch, args.count, args.start, args.end, - args.ignore_binary, args.add_signoff) + args.ignore_binary, args.add_signoff, + keep_change_id=args.keep_change_id) ok = check_patches(series, patch_files, args.check_patch, args.verbose, args.check_patch_use_tree) diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index f91669a..e2e2a83 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -68,6 +68,7 @@ STATE_PATCH_SUBJECT = 1 # In patch subject (first line of log for a commit) STATE_PATCH_HEADER = 2 # In patch header (after the subject) STATE_DIFFS = 3 # In the diff part (past --- line) + class PatchStream: """Class for detecting/injecting tags in a patch or series of patches @@ -76,7 +77,7 @@ class PatchStream: unwanted tags or inject additional ones. These correspond to the two phases of processing. """ - def __init__(self, series, is_log=False): + def __init__(self, series, is_log=False, keep_change_id=False): self.skip_blank = False # True to skip a single blank line self.found_test = False # Found a TEST= line self.lines_after_test = 0 # Number of lines found after TEST= @@ -86,6 +87,7 @@ class PatchStream: self.section = [] # The current section...END section self.series = series # Info about the patch series self.is_log = is_log # True if indent like git log + self.keep_change_id = keep_change_id # True to keep Change-Id tags self.in_change = None # Name of the change list we are in self.change_version = 0 # Non-zero if we are in a change list self.change_lines = [] # Lines of the current change @@ -452,6 +454,8 @@ class PatchStream: # Detect Change-Id tags elif change_id_match: + if self.keep_change_id: + out = [line] value = change_id_match.group(1) if self.is_log: if self.commit.change_id: @@ -763,7 +767,7 @@ def get_metadata_for_test(text): pst.finalise() return series -def fix_patch(backup_dir, fname, series, cmt): +def fix_patch(backup_dir, fname, series, cmt, keep_change_id=False): """Fix up a patch file, by adding/removing as required. We remove our tags from the patch file, insert changes lists, etc. @@ -776,6 +780,7 @@ def fix_patch(backup_dir, fname, series, cmt): fname (str): Filename to patch file to process series (Series): Series information about this patch set cmt (Commit): Commit object for this patch file + keep_change_id (bool): Keep the Change-Id tag. Return: list: A list of errors, each str, or [] if all ok. @@ -783,7 +788,7 @@ def fix_patch(backup_dir, fname, series, cmt): handle, tmpname = tempfile.mkstemp() outfd = os.fdopen(handle, 'w', encoding='utf-8') infd = open(fname, 'r', encoding='utf-8') - pst = PatchStream(series) + pst = PatchStream(series, keep_change_id=keep_change_id) pst.commit = cmt pst.process_stream(infd, outfd) infd.close() @@ -795,7 +800,7 @@ def fix_patch(backup_dir, fname, series, cmt): shutil.move(tmpname, fname) return cmt.warn -def fix_patches(series, fnames): +def fix_patches(series, fnames, keep_change_id=False): """Fix up a list of patches identified by filenames The patch files are processed in place, and overwritten. @@ -803,6 +808,7 @@ def fix_patches(series, fnames): Args: series (Series): The Series object fnames (:type: list of str): List of patch files to process + keep_change_id (bool): Keep the Change-Id tag. """ # Current workflow creates patches, so we shouldn't need a backup backup_dir = None #tempfile.mkdtemp('clean-patch') @@ -811,7 +817,8 @@ def fix_patches(series, fnames): cmt = series.commits[count] cmt.patch = fname cmt.count = count - result = fix_patch(backup_dir, fname, series, cmt) + result = fix_patch(backup_dir, fname, series, cmt, + keep_change_id=keep_change_id) if result: print('%d warning%s for %s:' % (len(result), 's' if len(result) > 1 else '', fname)) diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst index 038b651..a8b317e 100644 --- a/tools/patman/patman.rst +++ b/tools/patman/patman.rst @@ -371,11 +371,12 @@ Series-process-log: sort, uniq Separate each tag with a comma. Change-Id: - This tag is stripped out but is used to generate the Message-Id - of the emails that will be sent. When you keep the Change-Id the - same you are asserting that this is a slightly different version - (but logically the same patch) as other patches that have been - sent out with the same Change-Id. + This tag is used to generate the Message-Id of the emails that + will be sent. When you keep the Change-Id the same you are + asserting that this is a slightly different version (but logically + the same patch) as other patches that have been sent out with the + same Change-Id. The Change-Id tag line is removed from outgoing + patches, unless the `keep_change_id` settings is set to `True`. Various other tags are silently removed, like these Chrome OS and Gerrit tags:: diff --git a/tools/patman/test_checkpatch.py b/tools/patman/test_checkpatch.py index 0a8f740..db7860f 100644 --- a/tools/patman/test_checkpatch.py +++ b/tools/patman/test_checkpatch.py @@ -209,6 +209,22 @@ Signed-off-by: Simon Glass rc = os.system('diff -u %s %s' % (inname, expname)) self.assertEqual(rc, 0) + os.remove(inname) + + # Test whether the keep_change_id settings works. + inhandle, inname = tempfile.mkstemp() + infd = os.fdopen(inhandle, 'w', encoding='utf-8') + infd.write(data) + infd.close() + + patchstream.fix_patch(None, inname, series.Series(), com, + keep_change_id=True) + + with open(inname, 'r') as f: + content = f.read() + self.assertIn( + 'Change-Id: I80fe1d0c0b7dd10aa58ce5bb1d9290b6664d5413', + content) os.remove(inname) os.remove(expname) -- cgit v1.1 From 823f5c3a02276067c583b2f31144095f2b3b41fc Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 14 Oct 2023 14:40:25 -0600 Subject: binman: Reset missing bintools after testing For tests which fake bintools being missing, we need to reset the list afterwards, to ensure that future tests do not also see the bintools as missing. Reset the list when processing is complete. Signed-off-by: Simon Glass Reviewed-by: Neha Malcom Francis --- tools/binman/control.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/binman/control.py b/tools/binman/control.py index c6d3205..2f00279 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -858,6 +858,8 @@ def Binman(args): data = state.GetFdtForEtype('u-boot-dtb').GetContents() elf.UpdateFile(*elf_params, data) + bintool.Bintool.set_missing_list(None) + # This can only be True if -M is provided, since otherwise binman # would have raised an error already if invalid: -- cgit v1.1 From fdd623a0e3801ec5bf3703ef4e82cbf519e9f08e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 14 Oct 2023 14:40:26 -0600 Subject: binman: Don't add compression attribute for uncompressed files cbfsutil changed to skip adding a compression attribute if there is no compression. Adjust the binman implementation to do the same. This mirrors commit 105cdf5625 in coreboot. Signed-off-by: Simon Glass Reviewed-by: Neha Malcom Francis --- tools/binman/cbfs_util.py | 13 ++++++++----- tools/binman/cbfs_util_test.py | 8 ++++---- tools/binman/ftest.py | 6 +++--- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index fc56b40..92d2add 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -333,7 +333,8 @@ class CbfsFile(object): if self.ftype == TYPE_STAGE: pass elif self.ftype == TYPE_RAW: - hdr_len += ATTR_COMPRESSION_LEN + if self.compress: + hdr_len += ATTR_COMPRESSION_LEN elif self.ftype == TYPE_EMPTY: pass else: @@ -369,9 +370,11 @@ class CbfsFile(object): data = self.comp_bintool.compress(orig_data) self.memlen = len(orig_data) self.data_len = len(data) - attr = struct.pack(ATTR_COMPRESSION_FORMAT, - FILE_ATTR_TAG_COMPRESSION, ATTR_COMPRESSION_LEN, - self.compress, self.memlen) + if self.compress: + attr = struct.pack(ATTR_COMPRESSION_FORMAT, + FILE_ATTR_TAG_COMPRESSION, + ATTR_COMPRESSION_LEN, self.compress, + self.memlen) elif self.ftype == TYPE_EMPTY: data = tools.get_bytes(self.erase_byte, self.size) else: @@ -405,7 +408,7 @@ class CbfsFile(object): if expected_len != actual_len: # pragma: no cover # Test coverage of this is not available since this should never # happen. It probably indicates that get_header_len() is broken. - raise ValueError("Internal error: CBFS file '%s': Expected headers of %#x bytes, got %#d" % + raise ValueError("Internal error: CBFS file '%s': Expected headers of %#x bytes, got %#x" % (self.name, expected_len, actual_len)) return hdr + name + attr + pad + content + data, hdr_len diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py index ee951d1..c3babbc 100755 --- a/tools/binman/cbfs_util_test.py +++ b/tools/binman/cbfs_util_test.py @@ -96,7 +96,7 @@ class TestCbfs(unittest.TestCase): self.assertEqual(arch, cbfs.arch) return cbfs - def _check_uboot(self, cbfs, ftype=cbfs_util.TYPE_RAW, offset=0x38, + def _check_uboot(self, cbfs, ftype=cbfs_util.TYPE_RAW, offset=0x28, data=U_BOOT_DATA, cbfs_offset=None): """Check that the U-Boot file is as expected @@ -122,7 +122,7 @@ class TestCbfs(unittest.TestCase): self.assertEqual(len(data), cfile.memlen) return cfile - def _check_dtb(self, cbfs, offset=0x38, data=U_BOOT_DTB_DATA, + def _check_dtb(self, cbfs, offset=0x28, data=U_BOOT_DTB_DATA, cbfs_offset=None): """Check that the U-Boot dtb file is as expected @@ -598,8 +598,8 @@ class TestCbfs(unittest.TestCase): data = cbw.get_data() cbfs = cbfs_util.CbfsReader(data) - self.assertEqual(0x38, cbfs.files['u-boot'].cbfs_offset) - self.assertEqual(0x78, cbfs.files['u-boot-dtb'].cbfs_offset) + self.assertEqual(0x28, cbfs.files['u-boot'].cbfs_offset) + self.assertEqual(0x68, cbfs.files['u-boot-dtb'].cbfs_offset) if __name__ == '__main__': diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 16156b7..2875194 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2670,9 +2670,9 @@ class TestFunctional(unittest.TestCase): 'cbfs/u-boot:offset': 0x38, 'cbfs/u-boot:uncomp-size': len(U_BOOT_DATA), 'cbfs/u-boot:image-pos': 0x38, - 'cbfs/u-boot-dtb:offset': 0xb8, + 'cbfs/u-boot-dtb:offset': 0xa8, 'cbfs/u-boot-dtb:size': len(U_BOOT_DATA), - 'cbfs/u-boot-dtb:image-pos': 0xb8, + 'cbfs/u-boot-dtb:image-pos': 0xa8, }, props) def testCbfsBadType(self): @@ -2854,7 +2854,7 @@ class TestFunctional(unittest.TestCase): ' u-boot 0 4 u-boot 0', ' section 100 %x section 100' % section_size, ' cbfs 100 400 cbfs 0', -' u-boot 138 4 u-boot 38', +' u-boot 128 4 u-boot 28', ' u-boot-dtb 180 105 u-boot-dtb 80 3c9', ' u-boot-dtb 500 %x u-boot-dtb 400 3c9' % fdt_size, ' fdtmap %x 3bd fdtmap %x' % -- cgit v1.1 From 27f42bd5740cc55a919920a3e28bf9ac3936d1f6 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 14 Oct 2023 14:40:27 -0600 Subject: binman: Ensure attributes always come last in the metadata cbfsutil changed to write zero bytes instead of 0xff when a small padding must be added. Adjust the binman implementation to do the same. Drop the code which looks for an unused attribute tag, since it is not used. A future patch moves the attributes to the end of the header in any case, so no data will follow the attributes. This mirrors commit f0cc7adb2f in coreboot. Signed-off-by: Simon Glass --- tools/binman/cbfs_util.py | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 92d2add..9ac38d8 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -54,10 +54,6 @@ ATTR_COMPRESSION_FORMAT = '>IIII' ATTR_COMPRESSION_LEN = 0x10 # Attribute tags -# Depending on how the header was initialised, it may be backed with 0x00 or -# 0xff. Support both. -FILE_ATTR_TAG_UNUSED = 0 -FILE_ATTR_TAG_UNUSED2 = 0xffffffff FILE_ATTR_TAG_COMPRESSION = 0x42435a4c FILE_ATTR_TAG_HASH = 0x68736148 FILE_ATTR_TAG_POSITION = 0x42435350 # PSCB @@ -394,6 +390,8 @@ class CbfsFile(object): raise ValueError("Internal error: CBFS file '%s': Requested offset %#x but current output position is %#x" % (self.name, self.cbfs_offset, offset)) pad = tools.get_bytes(pad_byte, pad_len) + if attr_pos: + attr_pos += pad_len hdr_len += pad_len # This is the offset of the start of the file's data, @@ -410,7 +408,7 @@ class CbfsFile(object): # happen. It probably indicates that get_header_len() is broken. raise ValueError("Internal error: CBFS file '%s': Expected headers of %#x bytes, got %#x" % (self.name, expected_len, actual_len)) - return hdr + name + attr + pad + content + data, hdr_len + return hdr + name + pad + attr + content + data, hdr_len class CbfsWriter(object): @@ -456,6 +454,9 @@ class CbfsWriter(object): self._arch = arch self._bootblock_size = 0 self._erase_byte = 0xff + + # Small padding to align a file uses 0 + self._small_pad_byte = 0 self._align = ENTRY_ALIGN self._add_fileheader = False if self._arch == ARCHITECTURE_X86: @@ -477,7 +478,7 @@ class CbfsWriter(object): self._bootblock_size, self._align) self._hdr_at_start = True - def _skip_to(self, fd, offset): + def _skip_to(self, fd, offset, pad_byte): """Write out pad bytes until a given offset Args: @@ -487,16 +488,16 @@ class CbfsWriter(object): if fd.tell() > offset: raise ValueError('No space for data before offset %#x (current offset %#x)' % (offset, fd.tell())) - fd.write(tools.get_bytes(self._erase_byte, offset - fd.tell())) + fd.write(tools.get_bytes(pad_byte, offset - fd.tell())) - def _pad_to(self, fd, offset): + def _pad_to(self, fd, offset, pad_byte): """Write out pad bytes and/or an empty file until a given offset Args: fd: File objext to write to offset: Offset to write to """ - self._align_to(fd, self._align) + self._align_to(fd, self._align, pad_byte) upto = fd.tell() if upto > offset: raise ValueError('No space for data before pad offset %#x (current offset %#x)' % @@ -505,9 +506,9 @@ class CbfsWriter(object): if todo: cbf = CbfsFile.empty(todo, self._erase_byte) fd.write(cbf.get_data_and_offset()[0]) - self._skip_to(fd, offset) + self._skip_to(fd, offset, pad_byte) - def _align_to(self, fd, align): + def _align_to(self, fd, align, pad_byte): """Write out pad bytes until a given alignment is reached This only aligns if the resulting output would not reach the end of the @@ -521,7 +522,7 @@ class CbfsWriter(object): """ offset = align_int(fd.tell(), align) if offset < self._size: - self._skip_to(fd, offset) + self._skip_to(fd, offset, pad_byte) def add_file_stage(self, name, data, cbfs_offset=None): """Add a new stage file to the CBFS @@ -571,7 +572,7 @@ class CbfsWriter(object): raise ValueError('No space for header at offset %#x (current offset %#x)' % (self._header_offset, fd.tell())) if not add_fileheader: - self._pad_to(fd, self._header_offset) + self._pad_to(fd, self._header_offset, self._erase_byte) hdr = struct.pack(HEADER_FORMAT, HEADER_MAGIC, HEADER_VERSION2, self._size, self._bootblock_size, self._align, self._contents_offset, self._arch, 0xffffffff) @@ -583,7 +584,7 @@ class CbfsWriter(object): fd.write(name) self._header_offset = fd.tell() fd.write(hdr) - self._align_to(fd, self._align) + self._align_to(fd, self._align, self._erase_byte) else: fd.write(hdr) @@ -600,24 +601,26 @@ class CbfsWriter(object): # THe header can go at the start in some cases if self._hdr_at_start: self._write_header(fd, add_fileheader=self._add_fileheader) - self._skip_to(fd, self._contents_offset) + self._skip_to(fd, self._contents_offset, self._erase_byte) # Write out each file for cbf in self._files.values(): # Place the file at its requested place, if any offset = cbf.calc_start_offset() if offset is not None: - self._pad_to(fd, align_int_down(offset, self._align)) + self._pad_to(fd, align_int_down(offset, self._align), + self._erase_byte) pos = fd.tell() - data, data_offset = cbf.get_data_and_offset(pos, self._erase_byte) + data, data_offset = cbf.get_data_and_offset(pos, + self._small_pad_byte) fd.write(data) - self._align_to(fd, self._align) + self._align_to(fd, self._align, self._erase_byte) cbf.calced_cbfs_offset = pos + data_offset if not self._hdr_at_start: self._write_header(fd, add_fileheader=self._add_fileheader) # Pad to the end and write a pointer to the CBFS master header - self._pad_to(fd, self._base_address or self._size - 4) + self._pad_to(fd, self._base_address or self._size - 4, self._erase_byte) rel_offset = self._header_offset - self._size fd.write(struct.pack(' Date: Sat, 14 Oct 2023 14:40:28 -0600 Subject: binman: Replace FILENAME_ALIGN 16 with ATTRIBUTE_ALIGN 4 cbfsutil changed to 4-byte alignment for filenames instead of 16. Adjust the binman implementation to do the same. This mirrors commit 5779ca718c in coreboot. Signed-off-by: Simon Glass --- tools/binman/cbfs_util.py | 10 +++++----- tools/binman/cbfs_util_test.py | 19 ++++++++++--------- tools/binman/ftest.py | 10 +++++----- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 9ac38d8..076768a 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -42,7 +42,7 @@ HEADER_VERSION2 = 0x31313132 FILE_HEADER_FORMAT = b'>8sIIII' FILE_HEADER_LEN = 0x18 FILE_MAGIC = b'LARCHIVE' -FILENAME_ALIGN = 16 # Filename lengths are aligned to this +ATTRIBUTE_ALIGN = 4 # All attribute sizes must be divisible by this # A stage header containing information about 'stage' files # Yes this is correct: this header is in litte-endian format @@ -186,7 +186,7 @@ def _pack_string(instr): String with required padding (at least one 0x00 byte) at the end """ val = tools.to_bytes(instr) - pad_len = align_int(len(val) + 1, FILENAME_ALIGN) + pad_len = align_int(len(val) + 1, ATTRIBUTE_ALIGN) return val + tools.get_bytes(0, pad_len - len(val)) @@ -300,7 +300,7 @@ class CbfsFile(object): CbfsFile object containing the file information """ cfile = CbfsFile('', TYPE_EMPTY, b'', None) - cfile.size = space_to_use - FILE_HEADER_LEN - FILENAME_ALIGN + cfile.size = space_to_use - FILE_HEADER_LEN - ATTRIBUTE_ALIGN cfile.erase_byte = erase_byte return cfile @@ -859,8 +859,8 @@ class CbfsReader(object): """ val = b'' while True: - data = fd.read(FILENAME_ALIGN) - if len(data) < FILENAME_ALIGN: + data = fd.read(ATTRIBUTE_ALIGN) + if len(data) < ATTRIBUTE_ALIGN: return None pos = data.find(b'\0') if pos == -1: diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py index c3babbc..6459252 100755 --- a/tools/binman/cbfs_util_test.py +++ b/tools/binman/cbfs_util_test.py @@ -96,7 +96,7 @@ class TestCbfs(unittest.TestCase): self.assertEqual(arch, cbfs.arch) return cbfs - def _check_uboot(self, cbfs, ftype=cbfs_util.TYPE_RAW, offset=0x28, + def _check_uboot(self, cbfs, ftype=cbfs_util.TYPE_RAW, offset=0x20, data=U_BOOT_DATA, cbfs_offset=None): """Check that the U-Boot file is as expected @@ -122,7 +122,7 @@ class TestCbfs(unittest.TestCase): self.assertEqual(len(data), cfile.memlen) return cfile - def _check_dtb(self, cbfs, offset=0x28, data=U_BOOT_DTB_DATA, + def _check_dtb(self, cbfs, offset=0x24, data=U_BOOT_DTB_DATA, cbfs_offset=None): """Check that the U-Boot dtb file is as expected @@ -437,8 +437,9 @@ class TestCbfs(unittest.TestCase): pos = fd.tell() # Create a new CBFS with only the first 4 bytes of the compression tag, - # then try to read the file - tag_pos = pos + cbfs_util.FILE_HEADER_LEN + cbfs_util.FILENAME_ALIGN + # then try to read the file. Note that the tag gets pushed out 4 bytes + tag_pos = (4 + pos + cbfs_util.FILE_HEADER_LEN + + cbfs_util.ATTRIBUTE_ALIGN) newdata = data[:tag_pos + 4] with test_util.capture_sys_output() as (stdout, _stderr): with io.BytesIO(newdata) as fd: @@ -489,7 +490,7 @@ class TestCbfs(unittest.TestCase): load = 0xfef20000 entry = load + 2 - cfile = self._check_uboot(cbfs, cbfs_util.TYPE_STAGE, offset=0x28, + cfile = self._check_uboot(cbfs, cbfs_util.TYPE_STAGE, offset=0x20, data=U_BOOT_DATA + U_BOOT_DTB_DATA) self.assertEqual(entry, cfile.entry) @@ -520,7 +521,7 @@ class TestCbfs(unittest.TestCase): self.assertIn('u-boot', cbfs.files) cfile = cbfs.files['u-boot'] self.assertEqual(cfile.name, 'u-boot') - self.assertEqual(cfile.offset, 56) + self.assertEqual(cfile.offset, 0x30) self.assertEqual(cfile.data, COMPRESS_DATA) self.assertEqual(cfile.ftype, cbfs_util.TYPE_RAW) self.assertEqual(cfile.compress, cbfs_util.COMPRESS_LZ4) @@ -529,7 +530,7 @@ class TestCbfs(unittest.TestCase): self.assertIn('u-boot-dtb', cbfs.files) cfile = cbfs.files['u-boot-dtb'] self.assertEqual(cfile.name, 'u-boot-dtb') - self.assertEqual(cfile.offset, 56) + self.assertEqual(cfile.offset, 0x34) self.assertEqual(cfile.data, COMPRESS_DATA) self.assertEqual(cfile.ftype, cbfs_util.TYPE_RAW) self.assertEqual(cfile.compress, cbfs_util.COMPRESS_LZMA) @@ -598,8 +599,8 @@ class TestCbfs(unittest.TestCase): data = cbw.get_data() cbfs = cbfs_util.CbfsReader(data) - self.assertEqual(0x28, cbfs.files['u-boot'].cbfs_offset) - self.assertEqual(0x68, cbfs.files['u-boot-dtb'].cbfs_offset) + self.assertEqual(0x20, cbfs.files['u-boot'].cbfs_offset) + self.assertEqual(0x64, cbfs.files['u-boot-dtb'].cbfs_offset) if __name__ == '__main__': diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 2875194..5ace2a8 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2667,12 +2667,12 @@ class TestFunctional(unittest.TestCase): 'cbfs:offset': 0, 'cbfs:size': len(data), 'cbfs:image-pos': 0, - 'cbfs/u-boot:offset': 0x38, + 'cbfs/u-boot:offset': 0x30, 'cbfs/u-boot:uncomp-size': len(U_BOOT_DATA), - 'cbfs/u-boot:image-pos': 0x38, - 'cbfs/u-boot-dtb:offset': 0xa8, + 'cbfs/u-boot:image-pos': 0x30, + 'cbfs/u-boot-dtb:offset': 0xa4, 'cbfs/u-boot-dtb:size': len(U_BOOT_DATA), - 'cbfs/u-boot-dtb:image-pos': 0xa8, + 'cbfs/u-boot-dtb:image-pos': 0xa4, }, props) def testCbfsBadType(self): @@ -2854,7 +2854,7 @@ class TestFunctional(unittest.TestCase): ' u-boot 0 4 u-boot 0', ' section 100 %x section 100' % section_size, ' cbfs 100 400 cbfs 0', -' u-boot 128 4 u-boot 28', +' u-boot 120 4 u-boot 20', ' u-boot-dtb 180 105 u-boot-dtb 80 3c9', ' u-boot-dtb 500 %x u-boot-dtb 400 3c9' % fdt_size, ' fdtmap %x 3bd fdtmap %x' % -- cgit v1.1 From 20d8a8091f8a502f3257432e5f9e1e644cfc58f8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 14 Oct 2023 14:40:29 -0600 Subject: binman: Rename TYPE_STAGE to TYPE_LEGACY_STAGE In preparation for changing how stages are stored, rename the existing stage tag. Signed-off-by: Simon Glass --- cmd/cbfs.c | 2 +- include/cbfs.h | 2 +- tools/binman/cbfs_util.py | 12 ++++++------ tools/binman/cbfs_util_test.py | 3 ++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/cmd/cbfs.c b/cmd/cbfs.c index 8a61f2c..3cfc9eb 100644 --- a/cmd/cbfs.c +++ b/cmd/cbfs.c @@ -118,7 +118,7 @@ static int do_cbfs_ls(struct cmd_tbl *cmdtp, int flag, int argc, case CBFS_TYPE_CBFSHEADER: type_name = "cbfs header"; break; - case CBFS_TYPE_STAGE: + case CBFS_TYPE_LEGACY_STAGE: type_name = "stage"; break; case CBFS_TYPE_PAYLOAD: diff --git a/include/cbfs.h b/include/cbfs.h index 38efb1d..2bc5de2 100644 --- a/include/cbfs.h +++ b/include/cbfs.h @@ -22,7 +22,7 @@ enum cbfs_result { enum cbfs_filetype { CBFS_TYPE_BOOTBLOCK = 0x01, CBFS_TYPE_CBFSHEADER = 0x02, - CBFS_TYPE_STAGE = 0x10, + CBFS_TYPE_LEGACY_STAGE = 0x10, CBFS_TYPE_PAYLOAD = 0x20, CBFS_TYPE_SELF = CBFS_TYPE_PAYLOAD, diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 076768a..9ca32f7 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -96,7 +96,7 @@ ARCH_NAMES = { # File types. Only supported ones are included here TYPE_CBFSHEADER = 0x02 # Master header, HEADER_FORMAT -TYPE_STAGE = 0x10 # Stage, holding an executable, see STAGE_FORMAT +TYPE_LEGACY_STAGE = 0x10 # Stage, holding an executable TYPE_RAW = 0x50 # Raw file, possibly compressed TYPE_EMPTY = 0xffffffff # Empty data @@ -265,7 +265,7 @@ class CbfsFile(object): Returns: CbfsFile object containing the file information """ - cfile = CbfsFile(name, TYPE_STAGE, data, cbfs_offset) + cfile = CbfsFile(name, TYPE_LEGACY_STAGE, data, cbfs_offset) cfile.base_address = base_address return cfile @@ -326,7 +326,7 @@ class CbfsFile(object): """ name = _pack_string(self.name) hdr_len = len(name) + FILE_HEADER_LEN - if self.ftype == TYPE_STAGE: + if self.ftype == TYPE_LEGACY_STAGE: pass elif self.ftype == TYPE_RAW: if self.compress: @@ -354,7 +354,7 @@ class CbfsFile(object): attr = b'' pad = b'' data = self.data - if self.ftype == TYPE_STAGE: + if self.ftype == TYPE_LEGACY_STAGE: elf_data = elf.DecodeElf(data, self.base_address) content = struct.pack(STAGE_FORMAT, self.compress, elf_data.entry, elf_data.load, @@ -639,7 +639,7 @@ class CbfsReader(object): files: Ordered list of CbfsFile objects align: Alignment to use for files, typically ENTRT_ALIGN stage_base_address: Base address to use when mapping ELF files into the - CBFS for TYPE_STAGE files. If this is larger than the code address + CBFS for TYPE_LEGACY_STAGE files. If this is larger than the code address of the ELF file, then data at the start of the ELF file will not appear in the CBFS. Currently there are no tests for behaviour as documentation is sparse @@ -750,7 +750,7 @@ class CbfsReader(object): fd.seek(cbfs_offset, io.SEEK_SET) if ftype == TYPE_CBFSHEADER: self._read_header(fd) - elif ftype == TYPE_STAGE: + elif ftype == TYPE_LEGACY_STAGE: data = fd.read(STAGE_LEN) cfile = CbfsFile.stage(self.stage_base_address, name, b'', cbfs_offset) diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py index 6459252..2775d05 100755 --- a/tools/binman/cbfs_util_test.py +++ b/tools/binman/cbfs_util_test.py @@ -490,7 +490,8 @@ class TestCbfs(unittest.TestCase): load = 0xfef20000 entry = load + 2 - cfile = self._check_uboot(cbfs, cbfs_util.TYPE_STAGE, offset=0x20, + cfile = self._check_uboot(cbfs, cbfs_util.TYPE_LEGACY_STAGE, + offset=0x20, data=U_BOOT_DATA + U_BOOT_DTB_DATA) self.assertEqual(entry, cfile.entry) -- cgit v1.1 From 811cd3a41cb96f69726589ea1187611d98d6f4ca Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 14 Oct 2023 14:40:30 -0600 Subject: binman: Move stage header into a CBFS attribute cbfsutil completely changed the way that stages are formatted in CBFS. Adjust the binman implementation to do the same. This mirrors commit 81dc20e744 in coreboot. Signed-off-by: Simon Glass --- tools/binman/cbfs_util.py | 64 +++++++++++++++++++++++++----------------- tools/binman/cbfs_util_test.py | 7 ++--- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 9ca32f7..671cafa 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -44,10 +44,10 @@ FILE_HEADER_LEN = 0x18 FILE_MAGIC = b'LARCHIVE' ATTRIBUTE_ALIGN = 4 # All attribute sizes must be divisible by this -# A stage header containing information about 'stage' files +# A stage-header attribute containing information about 'stage' files # Yes this is correct: this header is in litte-endian format -STAGE_FORMAT = ' Date: Wed, 18 Oct 2023 15:40:12 +0200 Subject: cros_ec: spi: disable annoying key echo on console MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit on Peach-pi console every key press is echoed with message 'cros_ec_command: Returned status 1' this is not proper fix, just hack to disable this message Signed-off-by: Milan P. Stanić Reviewed-by: Simon Glass --- drivers/misc/cros_ec_spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/misc/cros_ec_spi.c b/drivers/misc/cros_ec_spi.c index 001f0a8..591ff30 100644 --- a/drivers/misc/cros_ec_spi.c +++ b/drivers/misc/cros_ec_spi.c @@ -151,7 +151,7 @@ int cros_ec_spi_command(struct udevice *udev, uint8_t cmd, int cmd_version, /* Response code is first byte of message */ if (p[0] != EC_RES_SUCCESS) { - printf("%s: Returned status %d\n", __func__, p[0]); + log_debug("Returned status %d\n", p[0]); return -(int)(p[0]); } -- cgit v1.1 From b2867fc121355271599eac20f785cb691089f806 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 23 Oct 2023 00:52:43 -0700 Subject: buildman: Include symbols in the read-only data section When symbols switch between the inited data section and the read-only data section their visbility changes, at present, with the -B option. This is confusing, since adding 'const' to a variable declaration can make it look like a significant improvement in bloat. But in fact nothing has changed. Add 'r' to the list of symbols types that are recorded, to correct this problem. Add a constant to make it easier to find this code next time. Signed-off-by: Simon Glass Reported-by: Tom Rini Reviewed-by: Tom Rini --- tools/buildman/builder.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 5305477..3e42c98 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -35,6 +35,10 @@ from u_boot_pylib.terminal import tprint # which indicates that BREAK_ME has an empty default RE_NO_DEFAULT = re.compile(b'\((\w+)\) \[] \(NEW\)') +# Symbol types which appear in the bloat feature (-B). Others are silently +# dropped when reading in the 'nm' output +NM_SYMBOL_TYPES = 'tTdDbBr' + """ Theory of Operation @@ -693,7 +697,7 @@ class Builder: parts = line.split() if line and len(parts) == 3: size, type, name = line.split() - if type in 'tTdDbB': + if type in NM_SYMBOL_TYPES: # function names begin with '.' on 64-bit powerpc if '.' in name[1:]: name = 'static.' + name.split('.')[0] -- cgit v1.1 From bf09cf52004735c587563400b230a7e27721253b Mon Sep 17 00:00:00 2001 From: Neha Malcom Francis Date: Mon, 23 Oct 2023 13:31:02 +0530 Subject: binman: openssl: x509: ti_secure_rom: Add support for bootcore_opts According to the TRMs of K3 platform of devices, the ROM boot image format specifies a "Core Options Field" that provides the capability to set the boot core in lockstep when set to 0 or to split mode when set to 2. Add support for providing the same from the binman DTS. Also modify existing test case for ensuring future coverage. Signed-off-by: Neha Malcom Francis Reviewed-by: Simon Glass --- tools/binman/btool/openssl.py | 6 ++++-- tools/binman/entries.rst | 1 + tools/binman/etype/ti_secure_rom.py | 11 +++++++++-- tools/binman/etype/x509_cert.py | 3 ++- tools/binman/test/297_ti_secure_rom.dts | 1 + 5 files changed, 17 insertions(+), 5 deletions(-) diff --git a/tools/binman/btool/openssl.py b/tools/binman/btool/openssl.py index aad3b61..7ee2683 100644 --- a/tools/binman/btool/openssl.py +++ b/tools/binman/btool/openssl.py @@ -155,6 +155,7 @@ authInPlace = INTEGER:2 C, ST, L, O, OU, CN and emailAddress cert_type (int): Certification type bootcore (int): Booting core + bootcore_opts(int): Booting core option, lockstep (0) or split (2) mode load_addr (int): Load address of image sha (int): Hash function @@ -225,7 +226,7 @@ emailAddress = {req_dist_name_dict['emailAddress']} imagesize_sbl, hashval_sbl, load_addr_sysfw, imagesize_sysfw, hashval_sysfw, load_addr_sysfw_data, imagesize_sysfw_data, hashval_sysfw_data, sysfw_inner_cert_ext_boot_block, - dm_data_ext_boot_block): + dm_data_ext_boot_block, bootcore_opts): """Create a certificate Args: @@ -241,6 +242,7 @@ emailAddress = {req_dist_name_dict['emailAddress']} bootcore (int): Booting core load_addr (int): Load address of image sha (int): Hash function + bootcore_opts (int): Booting core option, lockstep (0) or split (2) mode Returns: str: Tool output @@ -285,7 +287,7 @@ sysfw_data=SEQUENCE:sysfw_data [sbl] compType = INTEGER:1 bootCore = INTEGER:16 -compOpts = INTEGER:0 +compOpts = INTEGER:{bootcore_opts} destAddr = FORMAT:HEX,OCT:{load_addr:08x} compSize = INTEGER:{imagesize_sbl} shaType = OID:{sha_type} diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index e7b4e93..2402adb 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -1944,6 +1944,7 @@ Properties / Entry arguments: - core: core on which bootloader runs, valid cores are 'secure' and 'public' - content: phandle of SPL in case of legacy bootflow or phandles of component binaries in case of combined bootflow + - core-opts (optional): lockstep (0) or split (2) mode set to 0 by default The following properties are only for generating a combined bootflow binary: - sysfw-inner-cert: boolean if binary contains sysfw inner certificate diff --git a/tools/binman/etype/ti_secure_rom.py b/tools/binman/etype/ti_secure_rom.py index 9a7ac9e..f6fc3f9 100644 --- a/tools/binman/etype/ti_secure_rom.py +++ b/tools/binman/etype/ti_secure_rom.py @@ -32,6 +32,7 @@ class Entry_ti_secure_rom(Entry_x509_cert): - core: core on which bootloader runs, valid cores are 'secure' and 'public' - content: phandle of SPL in case of legacy bootflow or phandles of component binaries in case of combined bootflow + - core-opts (optional): lockstep (0) or split (2) mode set to 0 by default The following properties are only for generating a combined bootflow binary: - sysfw-inner-cert: boolean if binary contains sysfw inner certificate @@ -69,6 +70,7 @@ class Entry_ti_secure_rom(Entry_x509_cert): self.sw_rev = fdt_util.GetInt(self._node, 'sw-rev', 1) self.sha = fdt_util.GetInt(self._node, 'sha', 512) self.core = fdt_util.GetString(self._node, 'core', 'secure') + self.bootcore_opts = fdt_util.GetInt(self._node, 'core-opts') self.key_fname = self.GetEntryArgsOrProps([ EntryArg('keyfile', str)], required=True)[0] if self.combined: @@ -97,17 +99,19 @@ class Entry_ti_secure_rom(Entry_x509_cert): bytes content of the entry, which is the certificate binary for the provided data """ + if self.bootcore_opts is None: + self.bootcore_opts = 0 + if self.core == 'secure': if self.countersign: self.cert_type = 3 else: self.cert_type = 2 self.bootcore = 0 - self.bootcore_opts = 32 else: self.cert_type = 1 self.bootcore = 16 - self.bootcore_opts = 0 + return super().GetCertificate(required=required, type='rom') def CombinedGetCertificate(self, required): @@ -126,6 +130,9 @@ class Entry_ti_secure_rom(Entry_x509_cert): self.num_comps = 3 self.sha_type = SHA_OIDS[self.sha] + if self.bootcore_opts is None: + self.bootcore_opts = 0 + # sbl self.content = fdt_util.GetPhandleList(self._node, 'content-sbl') input_data_sbl = self.GetContents(required) diff --git a/tools/binman/etype/x509_cert.py b/tools/binman/etype/x509_cert.py index d028cfe..fc0bb12 100644 --- a/tools/binman/etype/x509_cert.py +++ b/tools/binman/etype/x509_cert.py @@ -136,7 +136,8 @@ class Entry_x509_cert(Entry_collection): imagesize_sysfw_data=self.imagesize_sysfw_data, hashval_sysfw_data=self.hashval_sysfw_data, sysfw_inner_cert_ext_boot_block=self.sysfw_inner_cert_ext_boot_block, - dm_data_ext_boot_block=self.dm_data_ext_boot_block + dm_data_ext_boot_block=self.dm_data_ext_boot_block, + bootcore_opts=self.bootcore_opts ) if stdout is not None: data = tools.read_file(output_fname) diff --git a/tools/binman/test/297_ti_secure_rom.dts b/tools/binman/test/297_ti_secure_rom.dts index d131376..1a3eca9 100644 --- a/tools/binman/test/297_ti_secure_rom.dts +++ b/tools/binman/test/297_ti_secure_rom.dts @@ -9,6 +9,7 @@ binman { ti-secure-rom { content = <&unsecure_binary>; + core-opts = <2>; }; unsecure_binary: blob-ext { filename = "ti_unsecure.bin"; -- cgit v1.1 From 625563211317562e58ab09a5f36c087c665ffa99 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 24 Oct 2023 08:30:47 +0200 Subject: sandbox: eliminate unused functions from binaries The sandbox should closely mimic other architectures. Place each function or data in a separate section and let the linker eliminate unused ones. This will reduce the binary size. In the linker script mark that u_boot_sandbox_getopt are to be kept. Signed-off-by: Heinrich Schuchardt Reviewed-by: Tom Rini --- arch/sandbox/config.mk | 4 ++-- arch/sandbox/cpu/u-boot.lds | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk index 2d184c5..1d50991 100644 --- a/arch/sandbox/config.mk +++ b/arch/sandbox/config.mk @@ -2,7 +2,7 @@ # Copyright (c) 2011 The Chromium OS Authors. PLATFORM_CPPFLAGS += -D__SANDBOX__ -U_FORTIFY_SOURCE -PLATFORM_CPPFLAGS += -fPIC +PLATFORM_CPPFLAGS += -fPIC -ffunction-sections -fdata-sections PLATFORM_LIBS += -lrt SDL_CONFIG ?= sdl2-config @@ -30,7 +30,7 @@ cmd_u-boot__ = $(CC) -o $@ -Wl,-T u-boot.lds $(u-boot-init) \ $(u-boot-main) \ $(u-boot-keep-syms-lto) \ -Wl,--no-whole-archive \ - $(PLATFORM_LIBS) -Wl,-Map -Wl,u-boot.map + $(PLATFORM_LIBS) -Wl,-Map -Wl,u-boot.map -Wl,--gc-sections cmd_u-boot-spl = (cd $(obj) && $(CC) -o $(SPL_BIN) -Wl,-T u-boot-spl.lds \ $(KBUILD_LDFLAGS:%=-Wl,%) \ diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds index ba8dee5..52f13af 100644 --- a/arch/sandbox/cpu/u-boot.lds +++ b/arch/sandbox/cpu/u-boot.lds @@ -15,7 +15,7 @@ SECTIONS _u_boot_sandbox_getopt : { *(_u_boot_sandbox_getopt_start) - *(_u_boot_sandbox_getopt) + KEEP(*(_u_boot_sandbox_getopt)) *(_u_boot_sandbox_getopt_end) } -- cgit v1.1 From e1387e66b4fb8bdde66ec6ee691e3d3ec8db71d6 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 30 Oct 2023 10:22:30 -0700 Subject: buildman: Support upstream branch name containing / Buildman assumes that branch names do not have a slash in them, since slash is used to delimit remotes, etc. This means that a branch called 'WIP/tryme' in remote dm ends up being 'tryme'. Adjust the logic a little, to try to accommodate this. For now, no tests are added for this behaviour. Signed-off-by: Simon Glass Reviewed-by: Tom Rini --- tools/patman/gitutil.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py index b0a12f2..10ea5ff 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -147,8 +147,9 @@ def get_upstream(git_dir, branch): if remote == '.': return merge, None elif remote and merge: - leaf = merge.split('/')[-1] - return '%s/%s' % (remote, leaf), None + # Drop the initial refs/heads from merge + leaf = merge.split('/', maxsplit=2)[2:] + return '%s/%s' % (remote, '/'.join(leaf)), None else: raise ValueError("Cannot determine upstream branch for branch " "'%s' remote='%s', merge='%s'" -- cgit v1.1 From 3227baad05481e37d75aeae7c7c1467fd6903f7d Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 1 Nov 2023 11:17:50 -0600 Subject: u_boot_pylib: Ensure subprocess is closed down It isn't clear why we need to have two different paths for closing down the pipe. Unify them and use the Python to avoid this warning: subprocess.py:1127: ResourceWarning: subprocess 83531 is still running Note that this code appears to originally have come from [1] and was committed into the ChromeOS chromiumos/platform/crosutils repo in the bin/cros_image_to_target.py file. The addition of the extra code path came later, so that is chosen for the fixes tag. [1] https://codereview.chromium.org/3391008 Signed-off-by: Simon Glass Fixes: a10fd93cbc patman: Make command methods return a CommandResult --- tools/u_boot_pylib/command.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/u_boot_pylib/command.py b/tools/u_boot_pylib/command.py index 9bbfc5b..bbe95d8 100644 --- a/tools/u_boot_pylib/command.py +++ b/tools/u_boot_pylib/command.py @@ -105,9 +105,7 @@ def run_pipe(pipe_list, infile=None, outfile=None, last_pipe.communicate_filter(output_func)) if result.stdout and oneline: result.output = result.stdout.rstrip(b'\r\n') - result.return_code = last_pipe.wait() - else: - result.return_code = os.waitpid(last_pipe.pid, 0)[1] + result.return_code = last_pipe.wait() if raise_on_error and result.return_code: raise Exception("Error running '%s'" % user_pipestr) return result.to_output(binary) -- cgit v1.1