From b50e5611a6b5dff4bc2ae47d332ba0d046e2a782 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:54:54 -0700 Subject: [PATCH 01/17] binman: Add a function to read ELF symbols In some cases we need to read symbols from U-Boot. At present we have a a few cases which does this via 'nm' and 'grep'. It is better to use objdump since that tells us the size of the symbols and also whether it is weak or not. Add a new module which reads ELF information from files. Update existing uses of 'nm' to use this module. Signed-off-by: Simon Glass --- tools/binman/binman.py | 3 +- tools/binman/elf.py | 77 +++++++++++++++++++++ tools/binman/elf_test.py | 32 +++++++++ tools/binman/etype/u_boot_spl_bss_pad.py | 7 +- tools/binman/etype/u_boot_with_ucode_ptr.py | 9 ++- tools/binman/ftest.py | 7 ++ tools/binman/test/bss_data.c | 2 +- tools/binman/test/u_boot_ucode_ptr.c | 2 +- 8 files changed, 128 insertions(+), 11 deletions(-) create mode 100644 tools/binman/elf.py create mode 100644 tools/binman/elf_test.py diff --git a/tools/binman/binman.py b/tools/binman/binman.py index 3ccf25f1f884..81a613ddc407 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -33,6 +33,7 @@ def RunTests(): """Run the functional tests and any embedded doctests""" + import elf_test import entry_test import fdt_test import ftest @@ -50,7 +51,7 @@ def RunTests(): # 'entry' module. suite = unittest.TestLoader().loadTestsFromTestCase(entry_test.TestEntry) suite.run(result) - for module in (ftest.TestFunctional, fdt_test.TestFdt): + for module in (ftest.TestFunctional, fdt_test.TestFdt, elf_test.TestElf): suite = unittest.TestLoader().loadTestsFromTestCase(module) suite.run(result) diff --git a/tools/binman/elf.py b/tools/binman/elf.py new file mode 100644 index 000000000000..97208b179500 --- /dev/null +++ b/tools/binman/elf.py @@ -0,0 +1,77 @@ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass +# +# SPDX-License-Identifier: GPL-2.0+ +# +# Handle various things related to ELF images +# + +from collections import namedtuple, OrderedDict +import command +import os +import re +import struct + +import tools + +Symbol = namedtuple('Symbol', ['section', 'address', 'size', 'weak']) + +# Used for tests which don't have an ELF file to read +ignore_missing_files = False + + +def GetSymbols(fname, patterns): + """Get the symbols from an ELF file + + Args: + fname: Filename of the ELF file to read + patterns: List of regex patterns to search for, each a string + + Returns: + None, if the file does not exist, or Dict: + key: Name of symbol + value: Hex value of symbol + """ + stdout = command.Output('objdump', '-t', fname, raise_on_error=False) + lines = stdout.splitlines() + if patterns: + re_syms = re.compile('|'.join(patterns)) + else: + re_syms = None + syms = {} + syms_started = False + for line in lines: + if not line or not syms_started: + if 'SYMBOL TABLE' in line: + syms_started = True + line = None # Otherwise code coverage complains about 'continue' + continue + if re_syms and not re_syms.search(line): + continue + + space_pos = line.find(' ') + value, rest = line[:space_pos], line[space_pos + 1:] + flags = rest[:7] + parts = rest[7:].split() + section, size = parts[:2] + if len(parts) > 2: + name = parts[2] + syms[name] = Symbol(section, int(value, 16), int(size,16), + flags[1] == 'w') + return syms + +def GetSymbolAddress(fname, sym_name): + """Get a value of a symbol from an ELF file + + Args: + fname: Filename of the ELF file to read + patterns: List of regex patterns to search for, each a string + + Returns: + Symbol value (as an integer) or None if not found + """ + syms = GetSymbols(fname, [sym_name]) + sym = syms.get(sym_name) + if not sym: + return None + return sym.address diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py new file mode 100644 index 000000000000..dca7bc59f991 --- /dev/null +++ b/tools/binman/elf_test.py @@ -0,0 +1,32 @@ +# +# Copyright (c) 2017 Google, Inc +# Written by Simon Glass +# +# SPDX-License-Identifier: GPL-2.0+ +# +# Test for the elf module + +import os +import sys +import unittest + +import elf + +binman_dir = os.path.dirname(os.path.realpath(sys.argv[0])) +fname = os.path.join(binman_dir, 'test', 'u_boot_ucode_ptr') + +class TestElf(unittest.TestCase): + def testAllSymbols(self): + syms = elf.GetSymbols(fname, []) + self.assertIn('.ucode', syms) + + def testRegexSymbols(self): + syms = elf.GetSymbols(fname, ['ucode']) + self.assertIn('.ucode', syms) + syms = elf.GetSymbols(fname, ['missing']) + self.assertNotIn('.ucode', syms) + syms = elf.GetSymbols(fname, ['missing', 'ucode']) + self.assertIn('.ucode', syms) + +if __name__ == '__main__': + unittest.main() diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py b/tools/binman/etype/u_boot_spl_bss_pad.py index c005f28191fe..c37f61db2355 100644 --- a/tools/binman/etype/u_boot_spl_bss_pad.py +++ b/tools/binman/etype/u_boot_spl_bss_pad.py @@ -9,6 +9,7 @@ # import command +import elf from entry import Entry from blob import Entry_blob import tools @@ -19,8 +20,8 @@ def __init__(self, image, etype, node): def ObtainContents(self): fname = tools.GetInputFilename('spl/u-boot-spl') - args = [['nm', fname], ['grep', '__bss_size']] - out = command.RunPipe(args, capture=True).stdout.splitlines() - bss_size = int(out[0].split()[0], 16) + bss_size = elf.GetSymbolAddress(fname, '__bss_size') + if not bss_size: + self.Raise('Expected __bss_size symbol in spl/u-boot-spl') self.data = chr(0) * bss_size self.contents_size = bss_size diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 6f01adb97018..99b437130dbb 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -9,6 +9,7 @@ import struct import command +import elf from entry import Entry from blob import Entry_blob import fdt_util @@ -31,11 +32,9 @@ def GetDefaultFilename(self): def ObtainContents(self): # Figure out where to put the microcode pointer fname = tools.GetInputFilename(self.elf_fname) - args = [['nm', fname], ['grep', '-w', '_dt_ucode_base_size']] - out = (command.RunPipe(args, capture=True, raise_on_error=False). - stdout.splitlines()) - if len(out) == 1: - self.target_pos = int(out[0].split()[0], 16) + sym = elf.GetSymbolAddress(fname, '_dt_ucode_base_size') + if sym: + self.target_pos = sym elif not fdt_util.GetBool(self._node, 'optional-ucode'): self.Raise('Cannot locate _dt_ucode_base_size symbol in u-boot') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 9083143894f0..c8155788afc8 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -835,6 +835,13 @@ def testSplBssPad(self): data = self._DoReadFile('47_spl_bss_pad.dts') self.assertEqual(U_BOOT_SPL_DATA + (chr(0) * 10) + U_BOOT_DATA, data) + with open(self.TestFile('u_boot_ucode_ptr')) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + with self.assertRaises(ValueError) as e: + data = self._DoReadFile('47_spl_bss_pad.dts') + self.assertIn('Expected __bss_size symbol in spl/u-boot-spl', + str(e.exception)) + def testPackStart16Spl(self): """Test that an image with an x86 start16 region can be created""" data = self._DoReadFile('48_x86-start16-spl.dts') diff --git a/tools/binman/test/bss_data.c b/tools/binman/test/bss_data.c index f865a9d9f673..e0305c382c35 100644 --- a/tools/binman/test/bss_data.c +++ b/tools/binman/test/bss_data.c @@ -4,7 +4,7 @@ * SPDX-License-Identifier: GPL-2.0+ * * Simple program to create a _dt_ucode_base_size symbol which can be read - * by 'nm'. This is used by binman tests. + * by binutils. This is used by binman tests. */ int bss_data[10]; diff --git a/tools/binman/test/u_boot_ucode_ptr.c b/tools/binman/test/u_boot_ucode_ptr.c index 24f349fb9e42..734d54f0d419 100644 --- a/tools/binman/test/u_boot_ucode_ptr.c +++ b/tools/binman/test/u_boot_ucode_ptr.c @@ -4,7 +4,7 @@ * SPDX-License-Identifier: GPL-2.0+ * * Simple program to create a _dt_ucode_base_size symbol which can be read - * by 'nm'. This is used by binman tests. + * by binutils. This is used by binman tests. */ static unsigned long _dt_ucode_base_size[2] From 47419eae4b266ec8d1954d9314d833d014d63ecd Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:54:55 -0700 Subject: [PATCH 02/17] binman: Add support for including spl/u-boot-spl.dtb This file contains the SPL device tree. Add support for including this by itself in images. Signed-off-by: Simon Glass --- tools/binman/etype/u_boot_spl_dtb.py | 17 +++++++++++++++++ tools/binman/ftest.py | 7 +++++++ tools/binman/test/51_u_boot_spl_dtb.dts | 13 +++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 tools/binman/etype/u_boot_spl_dtb.py create mode 100644 tools/binman/test/51_u_boot_spl_dtb.dts diff --git a/tools/binman/etype/u_boot_spl_dtb.py b/tools/binman/etype/u_boot_spl_dtb.py new file mode 100644 index 000000000000..6c5ce1e996b4 --- /dev/null +++ b/tools/binman/etype/u_boot_spl_dtb.py @@ -0,0 +1,17 @@ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass +# +# SPDX-License-Identifier: GPL-2.0+ +# +# Entry-type module for U-Boot device tree +# + +from entry import Entry +from blob import Entry_blob + +class Entry_u_boot_spl_dtb(Entry_blob): + def __init__(self, image, etype, node): + Entry_blob.__init__(self, image, etype, node) + + def GetDefaultFilename(self): + return 'spl/u-boot-spl.dtb' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index c8155788afc8..32bc7950b143 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -33,6 +33,7 @@ ME_DATA = '0abcd' VGA_DATA = 'vga' U_BOOT_DTB_DATA = 'udtb' +U_BOOT_SPL_DTB_DATA = 'spldtb' X86_START16_DATA = 'start16' X86_START16_SPL_DATA = 'start16spl' U_BOOT_NODTB_DATA = 'nodtb with microcode pointer somewhere in here' @@ -76,6 +77,7 @@ def setUpClass(self): TestFunctional._MakeInputFile('me.bin', ME_DATA) TestFunctional._MakeInputFile('vga.bin', VGA_DATA) TestFunctional._MakeInputFile('u-boot.dtb', U_BOOT_DTB_DATA) + TestFunctional._MakeInputFile('spl/u-boot-spl.dtb', U_BOOT_SPL_DTB_DATA) TestFunctional._MakeInputFile('u-boot-x86-16bit.bin', X86_START16_DATA) TestFunctional._MakeInputFile('spl/u-boot-x86-16bit-spl.bin', X86_START16_SPL_DATA) @@ -869,6 +871,11 @@ def testPackMrc(self): data = self._DoReadFile('50_intel_mrc.dts') self.assertEqual(MRC_DATA, data[:len(MRC_DATA)]) + def testSplDtb(self): + """Test that an image with spl/u-boot-spl.dtb can be created""" + data = self._DoReadFile('51_u_boot_spl_dtb.dts') + self.assertEqual(U_BOOT_SPL_DTB_DATA, data[:len(U_BOOT_SPL_DTB_DATA)]) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/51_u_boot_spl_dtb.dts b/tools/binman/test/51_u_boot_spl_dtb.dts new file mode 100644 index 000000000000..3912f86b4cd7 --- /dev/null +++ b/tools/binman/test/51_u_boot_spl_dtb.dts @@ -0,0 +1,13 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <16>; + + u-boot-spl-dtb { + }; + }; +}; From 4e6fdbef67e4e73b516c20b073cf160679940b77 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:54:56 -0700 Subject: [PATCH 03/17] binman: Add support for including spl/u-boot-spl-nodtb.bin This file contains SPL image without a device tree. Add support for including this in images. Signed-off-by: Simon Glass --- tools/binman/etype/u_boot_spl_nodtb.py | 17 +++++++++++++++++ tools/binman/ftest.py | 5 +++++ tools/binman/test/52_u_boot_spl_nodtb.dts | 11 +++++++++++ 3 files changed, 33 insertions(+) create mode 100644 tools/binman/etype/u_boot_spl_nodtb.py create mode 100644 tools/binman/test/52_u_boot_spl_nodtb.dts diff --git a/tools/binman/etype/u_boot_spl_nodtb.py b/tools/binman/etype/u_boot_spl_nodtb.py new file mode 100644 index 000000000000..880e0c78fbc0 --- /dev/null +++ b/tools/binman/etype/u_boot_spl_nodtb.py @@ -0,0 +1,17 @@ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass +# +# SPDX-License-Identifier: GPL-2.0+ +# +# Entry-type module for 'u-boot-nodtb.bin' +# + +from entry import Entry +from blob import Entry_blob + +class Entry_u_boot_spl_nodtb(Entry_blob): + def __init__(self, image, etype, node): + Entry_blob.__init__(self, image, etype, node) + + def GetDefaultFilename(self): + return 'spl/u-boot-spl-nodtb.bin' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 32bc7950b143..ed0697af0069 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -876,6 +876,11 @@ def testSplDtb(self): data = self._DoReadFile('51_u_boot_spl_dtb.dts') self.assertEqual(U_BOOT_SPL_DTB_DATA, data[:len(U_BOOT_SPL_DTB_DATA)]) + def testSplNoDtb(self): + """Test that an image with spl/u-boot-spl-nodtb.bin can be created""" + data = self._DoReadFile('52_u_boot_spl_nodtb.dts') + self.assertEqual(U_BOOT_SPL_NODTB_DATA, data[:len(U_BOOT_SPL_NODTB_DATA)]) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/52_u_boot_spl_nodtb.dts b/tools/binman/test/52_u_boot_spl_nodtb.dts new file mode 100644 index 000000000000..7f4e27780fe4 --- /dev/null +++ b/tools/binman/test/52_u_boot_spl_nodtb.dts @@ -0,0 +1,11 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-spl-nodtb { + }; + }; +}; From 00ae40b3ae914150485bb8c74df0c0ecf689c7b7 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:54:57 -0700 Subject: [PATCH 04/17] binman: Drop a stale comment about the 'board' feature This feature is now supported. Drop the incorrect comment. Signed-off-by: Simon Glass --- tools/binman/ftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index ed0697af0069..590299da8bcb 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -304,7 +304,6 @@ def testHelp(self): self.assertEqual(0, len(result.stderr)) self.assertEqual(0, result.return_code) - # Not yet available. def testBoard(self): """Test that we can run it with a specific board""" self._SetupDtb('05_simple.dts', 'sandbox/u-boot.dtb') From 5cfcf7e0fdb265672dd8ee7d3e881190ed06ff98 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:54:58 -0700 Subject: [PATCH 05/17] binman: Add tests binaries with binman symbols For testing we need to build some ELF files containing binman symbols. Add these to the Makefile and check in the binaries: u_boot_binman_syms - normal, valid ELF file u_boot_binman_syms_bad - missing the __image_copy_start symbol u_boot_binman_syms_size - has a binman symbol with an invalid size Signed-off-by: Simon Glass --- tools/binman/test/Makefile | 18 ++++++++++- tools/binman/test/u_boot_binman_syms | Bin 0 -> 4921 bytes tools/binman/test/u_boot_binman_syms.c | 14 +++++++++ tools/binman/test/u_boot_binman_syms.lds | 30 +++++++++++++++++++ tools/binman/test/u_boot_binman_syms_bad | Bin 0 -> 4890 bytes tools/binman/test/u_boot_binman_syms_bad.c | 1 + tools/binman/test/u_boot_binman_syms_bad.lds | 29 ++++++++++++++++++ tools/binman/test/u_boot_binman_syms_size | Bin 0 -> 4825 bytes tools/binman/test/u_boot_binman_syms_size.c | 12 ++++++++ 9 files changed, 103 insertions(+), 1 deletion(-) create mode 100755 tools/binman/test/u_boot_binman_syms create mode 100644 tools/binman/test/u_boot_binman_syms.c create mode 100644 tools/binman/test/u_boot_binman_syms.lds create mode 100755 tools/binman/test/u_boot_binman_syms_bad create mode 120000 tools/binman/test/u_boot_binman_syms_bad.c create mode 100644 tools/binman/test/u_boot_binman_syms_bad.lds create mode 100755 tools/binman/test/u_boot_binman_syms_size create mode 100644 tools/binman/test/u_boot_binman_syms_size.c diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index 217d13c666f1..e58fc807757b 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -10,8 +10,12 @@ CFLAGS := -march=i386 -m32 -nostdlib -I ../../../include LDS_UCODE := -T u_boot_ucode_ptr.lds +LDS_BINMAN := -T u_boot_binman_syms.lds +LDS_BINMAN_BAD := -T u_boot_binman_syms_bad.lds -TARGETS = u_boot_ucode_ptr u_boot_no_ucode_ptr bss_data +TARGETS = u_boot_ucode_ptr u_boot_no_ucode_ptr bss_data \ + u_boot_binman_syms u_boot_binman_syms.bin u_boot_binman_syms_bad \ + u_boot_binman_syms_size all: $(TARGETS) @@ -24,6 +28,18 @@ u_boot_ucode_ptr: u_boot_ucode_ptr.c bss_data: CFLAGS += bss_data.lds bss_data: bss_data.c +u_boot_binman_syms.bin: u_boot_binman_syms + objcopy -O binary $< -R .note.gnu.build-id $@ + +u_boot_binman_syms: CFLAGS += $(LDS_BINMAN) +u_boot_binman_syms: u_boot_binman_syms.c + +u_boot_binman_syms_bad: CFLAGS += $(LDS_BINMAN_BAD) +u_boot_binman_syms_bad: u_boot_binman_syms_bad.c + +u_boot_binman_syms_size: CFLAGS += $(LDS_BINMAN) +u_boot_binman_syms_size: u_boot_binman_syms_size.c + clean: rm -f $(TARGETS) diff --git a/tools/binman/test/u_boot_binman_syms b/tools/binman/test/u_boot_binman_syms new file mode 100755 index 0000000000000000000000000000000000000000..2e02dc0ca9e2651bc12ac6d61d95dfa5ccee7a68 GIT binary patch literal 4921 zcmeHLJ!=$E6up~WjCLCl8=FP8Sdlz7>lA8b2$~2I5R9ddncX2S?0m5ECa{euEN#TX z-bUgtNGn*`S@{qA0oE3J&dyvj`LMBm7tY*|bKjl!4%5s#eE8&1tJM;<6={oR0g6Z6 ziV36#W+1E5srb51mVLw}Ca8C6Pe<$5V4ZmS!%g7M8P_+)p5uMNE8rFI3U~#)0$u^H zfLFjP;1%!+cm=!yUV;BpfdAvyfHttbyC*l3%`dz8*Q1?-H{tP%@4r60d-n0{=O~TY-jYab;7BkM0Bu?GgE&MaT0cI6g(GFZF^fiX(d1(gVJqOQCzK_1<-GlZ$ z0LQ)|Knu`xy9$?2a>N?IS!x!2k2Qg34~_os-VlGr!f)`r`ylvUER`!jbKcvlBA-xG z(|MX-2Q8^aXJy(EY!p8Gzs@(JI?Y&{k}g(dWGTu literal 0 HcmV?d00001 diff --git a/tools/binman/test/u_boot_binman_syms.c b/tools/binman/test/u_boot_binman_syms.c new file mode 100644 index 000000000000..a9754769441c --- /dev/null +++ b/tools/binman/test/u_boot_binman_syms.c @@ -0,0 +1,14 @@ +/* + * Copyright (c) 2017 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + * + * Simple program to create some binman symbols. This is used by binman tests. + */ + +#define CONFIG_BINMAN +#include + +binman_sym_declare(unsigned long, u_boot_spl, pos); +binman_sym_declare(unsigned long long, u_boot_spl2, pos); +binman_sym_declare(unsigned long, u_boot_any, pos); diff --git a/tools/binman/test/u_boot_binman_syms.lds b/tools/binman/test/u_boot_binman_syms.lds new file mode 100644 index 000000000000..d3130cdeb316 --- /dev/null +++ b/tools/binman/test/u_boot_binman_syms.lds @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2016 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386") +OUTPUT_ARCH(i386) +ENTRY(_start) + +SECTIONS +{ + . = 0x00000000; + _start = .; + + . = ALIGN(4); + .text : + { + __image_copy_start = .; + *(.text*) + } + + . = ALIGN(4); + .binman_sym_table : { + __binman_sym_start = .; + KEEP(*(SORT(.binman_sym*))); + __binman_sym_end = .; + } + +} diff --git a/tools/binman/test/u_boot_binman_syms_bad b/tools/binman/test/u_boot_binman_syms_bad new file mode 100755 index 0000000000000000000000000000000000000000..8da3d9d48f388a9be53e92590984411691f6721f GIT binary patch literal 4890 zcmeHLJ!=$E6up~WjOjLFrOg&wtVkY797F^`2yqpRVv#}#JZ3VBF6_=MJ8y(ENwASv zTZuow-an8(k|ITFk^BHXXW!gp@?m5BF5I~v=e#@bo!MsJ-ulaDjYdP%=A?|UEab!>$^ba;d9Esg+fF-hI&dUhZQ7UZ1yrOXM4R zYC2Er>!RM|@O-r9g*UTShR0j-`;X7g>pMufp8HzF`iCBxJ=-|V6J$O3O*rv)h}25? zACdEJh}H)F8BzDcT1uPbxwGeAzOYH0nr+cmMJOgCJDKJaJIM>Ng^Q=|8p>*oQ;n?F U$JtH|)8YK34YE{hz2S%d1wo~NOaK4? literal 0 HcmV?d00001 diff --git a/tools/binman/test/u_boot_binman_syms_bad.c b/tools/binman/test/u_boot_binman_syms_bad.c new file mode 120000 index 000000000000..939b2e965f8d --- /dev/null +++ b/tools/binman/test/u_boot_binman_syms_bad.c @@ -0,0 +1 @@ +u_boot_binman_syms.c \ No newline at end of file diff --git a/tools/binman/test/u_boot_binman_syms_bad.lds b/tools/binman/test/u_boot_binman_syms_bad.lds new file mode 100644 index 000000000000..0b474b537479 --- /dev/null +++ b/tools/binman/test/u_boot_binman_syms_bad.lds @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2016 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386") +OUTPUT_ARCH(i386) +ENTRY(_start) + +SECTIONS +{ + . = 0x00000000; + _start = .; + + . = ALIGN(4); + .text : + { + *(.text*) + } + + . = ALIGN(4); + .binman_sym_table : { + __binman_sym_start = .; + KEEP(*(SORT(.binman_sym*))); + __binman_sym_end = .; + } + +} diff --git a/tools/binman/test/u_boot_binman_syms_size b/tools/binman/test/u_boot_binman_syms_size new file mode 100755 index 0000000000000000000000000000000000000000..d691e897c0f1842cb82efbc67f57d9f62853b99c GIT binary patch literal 4825 zcmeHLyKWOv5FI~+mChk_G%jq(2>Hq;D?t(oMY4z$2|-f0i*)7nuFOi_hrD+Sryze| zet;iH2NGo(I=XcH0Tm7Tf^f#WV{DlQ>O0c$+?hEuyZ6|Q=jzq#lTxWfVr8n3L=KW4 z>v_eY1}bf;Q8lj@d9Jn!Jm3KNYT?=D&rP=W*Agf^anP^*B!EMcZ%C C$!_BS literal 0 HcmV?d00001 diff --git a/tools/binman/test/u_boot_binman_syms_size.c b/tools/binman/test/u_boot_binman_syms_size.c new file mode 100644 index 000000000000..ee4c048b2883 --- /dev/null +++ b/tools/binman/test/u_boot_binman_syms_size.c @@ -0,0 +1,12 @@ +/* + * Copyright (c) 2017 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + * + * Simple program to create some binman symbols. This is used by binman tests. + */ + +#define CONFIG_BINMAN +#include + +binman_sym_declare(char, u_boot_spl, pos); From f689890d8ec52c8b9d005fbf7f6df00dcf9895db Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:54:59 -0700 Subject: [PATCH 06/17] binman: Adjust size of test SPL binary This is only 3 bytes long which is not enough to hold two symbol values, needed to test the binman symbols feature. Increase it to 15 bytes. Using very small regions is useful since we can easily compare them in tests and errors are fairly easy to diagnose. Signed-off-by: Simon Glass --- tools/binman/ftest.py | 12 ++++++------ tools/binman/test/21_image_pad.dts | 2 +- tools/binman/test/24_sorted.dts | 4 ++-- tools/binman/test/28_pack_4gb_outside.dts | 4 ++-- tools/binman/test/29_x86-rom.dts | 6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 590299da8bcb..372b61fbb3bf 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -28,7 +28,7 @@ # Contents of test files, corresponding to different entry types U_BOOT_DATA = '1234' U_BOOT_IMG_DATA = 'img' -U_BOOT_SPL_DATA = '567' +U_BOOT_SPL_DATA = '56780123456789abcde' BLOB_DATA = '89' ME_DATA = '0abcd' VGA_DATA = 'vga' @@ -565,7 +565,7 @@ def testPackAlignPowerOf2(self): def testImagePadByte(self): """Test that the image pad byte can be specified""" data = self._DoReadFile('21_image_pad.dts') - self.assertEqual(U_BOOT_SPL_DATA + (chr(0xff) * 9) + U_BOOT_DATA, data) + self.assertEqual(U_BOOT_SPL_DATA + (chr(0xff) * 1) + U_BOOT_DATA, data) def testImageName(self): """Test that image files can be named""" @@ -587,7 +587,7 @@ def testBlobFilename(self): def testPackSorted(self): """Test that entries can be sorted""" data = self._DoReadFile('24_sorted.dts') - self.assertEqual(chr(0) * 5 + U_BOOT_SPL_DATA + chr(0) * 2 + + self.assertEqual(chr(0) * 1 + U_BOOT_SPL_DATA + chr(0) * 2 + U_BOOT_DATA, data) def testPackZeroPosition(self): @@ -615,14 +615,14 @@ def testPackX86RomOutside(self): with self.assertRaises(ValueError) as e: self._DoTestFile('28_pack_4gb_outside.dts') self.assertIn("Node '/binman/u-boot': Position 0x0 (0) is outside " - "the image starting at 0xfffffff0 (4294967280)", + "the image starting at 0xffffffe0 (4294967264)", str(e.exception)) def testPackX86Rom(self): """Test that a basic x86 ROM can be created""" data = self._DoReadFile('29_x86-rom.dts') - self.assertEqual(U_BOOT_DATA + chr(0) * 3 + U_BOOT_SPL_DATA + - chr(0) * 6, data) + self.assertEqual(U_BOOT_DATA + chr(0) * 7 + U_BOOT_SPL_DATA + + chr(0) * 2, data) def testPackX86RomMeNoDesc(self): """Test that an invalid Intel descriptor entry is detected""" diff --git a/tools/binman/test/21_image_pad.dts b/tools/binman/test/21_image_pad.dts index daf8385f6d55..bf39dc1b6f71 100644 --- a/tools/binman/test/21_image_pad.dts +++ b/tools/binman/test/21_image_pad.dts @@ -10,7 +10,7 @@ }; u-boot { - pos = <12>; + pos = <20>; }; }; }; diff --git a/tools/binman/test/24_sorted.dts b/tools/binman/test/24_sorted.dts index 9f4151c932b0..43a7831341ca 100644 --- a/tools/binman/test/24_sorted.dts +++ b/tools/binman/test/24_sorted.dts @@ -7,11 +7,11 @@ binman { sort-by-pos; u-boot { - pos = <10>; + pos = <22>; }; u-boot-spl { - pos = <5>; + pos = <1>; }; }; }; diff --git a/tools/binman/test/28_pack_4gb_outside.dts b/tools/binman/test/28_pack_4gb_outside.dts index ff468c7d41d0..18d6bb5b8af5 100644 --- a/tools/binman/test/28_pack_4gb_outside.dts +++ b/tools/binman/test/28_pack_4gb_outside.dts @@ -7,13 +7,13 @@ binman { sort-by-pos; end-at-4gb; - size = <16>; + size = <32>; u-boot { pos = <0>; }; u-boot-spl { - pos = <0xfffffff7>; + pos = <0xffffffeb>; }; }; }; diff --git a/tools/binman/test/29_x86-rom.dts b/tools/binman/test/29_x86-rom.dts index 075ede36ab32..d49078e19e2d 100644 --- a/tools/binman/test/29_x86-rom.dts +++ b/tools/binman/test/29_x86-rom.dts @@ -7,13 +7,13 @@ binman { sort-by-pos; end-at-4gb; - size = <16>; + size = <32>; u-boot { - pos = <0xfffffff0>; + pos = <0xffffffe0>; }; u-boot-spl { - pos = <0xfffffff7>; + pos = <0xffffffeb>; }; }; }; From 7fe9173be78f32047bc38f2d68ac86e871dbfcae Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:55:00 -0700 Subject: [PATCH 07/17] binman: Support enabling debug in tests The elf module can provide some debugging information to assist with figuring out what is going wrong. This is also useful in tests. Update the -D option so that it is passed through to tests as well. Signed-off-by: Simon Glass --- tools/binman/binman.py | 6 ++++-- tools/binman/control.py | 3 +++ tools/binman/elf.py | 3 +++ tools/binman/ftest.py | 17 +++++++++++++---- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/tools/binman/binman.py b/tools/binman/binman.py index 81a613ddc407..aa5139626634 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -31,7 +31,7 @@ import command import control -def RunTests(): +def RunTests(debug): """Run the functional tests and any embedded doctests""" import elf_test import entry_test @@ -46,6 +46,8 @@ def RunTests(): suite.run(result) sys.argv = [sys.argv[0]] + if debug: + sys.argv.append('-D') # Run the entry tests first ,since these need to be the first to import the # 'entry' module. @@ -111,7 +113,7 @@ def RunBinman(options, args): sys.tracebacklimit = 0 if options.test: - ret_code = RunTests() + ret_code = RunTests(options.debug) elif options.test_coverage: RunTestCoverage() diff --git a/tools/binman/control.py b/tools/binman/control.py index e9d48df03014..e175e8d41bd0 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -12,6 +12,7 @@ import tools import command +import elf import fdt import fdt_util from image import Image @@ -89,6 +90,8 @@ def Binman(options, args): try: tout.Init(options.verbosity) + if options.debug: + elf.debug = True try: tools.SetInputDirs(options.indir) tools.PrepareOutputDir(options.outdir, options.preserve) diff --git a/tools/binman/elf.py b/tools/binman/elf.py index 97208b179500..0fb5a4a8ed63 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -14,6 +14,9 @@ import tools +# This is enabled from control.py +debug = False + Symbol = namedtuple('Symbol', ['section', 'address', 'size', 'weak']) # Used for tests which don't have an ELF file to read diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 372b61fbb3bf..2bee6a168f38 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -136,7 +136,10 @@ def _DoBinman(self, *args): Returns: Return value (0 for success) """ - (options, args) = cmdline.ParseArgs(list(args)) + args = list(args) + if '-D' in sys.argv: + args = args + ['-D'] + (options, args) = cmdline.ParseArgs(args) options.pager = 'binman-invalid-pager' options.build_dir = self._indir @@ -144,14 +147,16 @@ def _DoBinman(self, *args): # options.verbosity = tout.DEBUG return control.Binman(options, args) - def _DoTestFile(self, fname): + def _DoTestFile(self, fname, debug=False): """Run binman with a given test file Args: fname: Device tree source filename to use (e.g. 05_simple.dts) """ - return self._DoBinman('-p', '-I', self._indir, - '-d', self.TestFile(fname)) + args = ['-p', '-I', self._indir, '-d', self.TestFile(fname)] + if debug: + args.append('-D') + return self._DoBinman(*args) def _SetupDtb(self, fname, outfile='u-boot.dtb'): """Set up a new test device-tree file @@ -363,6 +368,10 @@ def testSimple(self): data = self._DoReadFile('05_simple.dts') self.assertEqual(U_BOOT_DATA, data) + def testSimpleDebug(self): + """Test a simple binman run with debugging enabled""" + data = self._DoTestFile('05_simple.dts', debug=True) + def testDual(self): """Test that we can handle creating two images From 19790632648be6fff7a4898350bd52565bde7c96 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:55:01 -0700 Subject: [PATCH 08/17] binman: Support accessing binman tables at run time Binman construct images consisting of multiple binary files. These files sometimes need to know (at run timme) where their peers are located. For example, SPL may want to know where U-Boot is located in the image, so that it can jump to U-Boot correctly on boot. In general the positions where the binaries end up after binman has finished packing them cannot be known at compile time. One reason for this is that binman does not know the size of the binaries until everything is compiled, linked and converted to binaries with objcopy. To make this work, we add a feature to binman which checks each binary for symbol names starting with '_binman'. These are then decoded to figure out which entry and property they refer to. Then binman writes the value of this symbol into the appropriate binary. With this, the symbol will have the correct value at run time. Macros are used to make this easier to use. As an example, this declares a symbol that will access the 'u-boot-spl' entry to find the 'pos' value (i.e. the position of SPL in the image): binman_sym_declare(unsigned long, u_boot_spl, pos); This converts to a symbol called '_binman_u_boot_spl_prop_pos' in any binary that includes it. Binman then updates the value in that binary, ensuring that it can be accessed at runtime with: ulong u_boot_pos = binman_sym(ulong, u_boot_spl, pos); This assigns the variable u_boot_pos to the position of SPL in the image. Signed-off-by: Simon Glass --- include/binman_sym.h | 80 +++++++++++++++++++++++++++ tools/binman/binman.py | 4 +- tools/binman/control.py | 4 +- tools/binman/elf.py | 55 +++++++++++++++++-- tools/binman/elf_test.py | 92 +++++++++++++++++++++++++++++++- tools/binman/etype/entry.py | 8 +++ tools/binman/etype/u_boot_spl.py | 6 +++ tools/binman/ftest.py | 19 +++++++ tools/binman/image.py | 79 +++++++++++++++++++++++++-- tools/binman/image_test.py | 46 ++++++++++++++++ tools/binman/test/53_symbols.dts | 20 +++++++ 11 files changed, 403 insertions(+), 10 deletions(-) create mode 100644 include/binman_sym.h create mode 100644 tools/binman/image_test.py create mode 100644 tools/binman/test/53_symbols.dts diff --git a/include/binman_sym.h b/include/binman_sym.h new file mode 100644 index 000000000000..3999b26d8d4f --- /dev/null +++ b/include/binman_sym.h @@ -0,0 +1,80 @@ +/* + * Symbol access for symbols set up by binman as part of the build. + * + * This allows C code to access the position of a particular part of the image + * assembled by binman. + * + * Copyright (c) 2017 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef __BINMAN_SYM_H +#define __BINMAN_SYM_H + +#define BINMAN_SYM_MISSING (-1UL) + +#ifdef CONFIG_BINMAN + +/** + * binman_symname() - Internal fnuction to get a binman symbol name + * + * @entry_name: Name of the entry to look for (e.g. 'u_boot_spl') + * @_prop_name: Property value to get from that entry (e.g. 'pos') + * @returns name of the symbol for that entry and property + */ +#define binman_symname(_entry_name, _prop_name) \ + _binman_ ## _entry_name ## _prop_ ## _prop_name + +/** + * binman_sym_declare() - Declare a symbol that will be used at run-time + * + * @_type: Type f the symbol (e.g. unsigned long) + * @entry_name: Name of the entry to look for (e.g. 'u_boot_spl') + * @_prop_name: Property value to get from that entry (e.g. 'pos') + */ +#define binman_sym_declare(_type, _entry_name, _prop_name) \ + _type binman_symname(_entry_name, _prop_name) \ + __attribute__((aligned(4), unused, section(".binman_sym"))) + +/** + * binman_sym_declare_optional() - Declare an optional symbol + * + * If this symbol cannot be provided by binman, an error will not be generated. + * Instead the image will be assigned the value BINMAN_SYM_MISSING. + * + * @_type: Type f the symbol (e.g. unsigned long) + * @entry_name: Name of the entry to look for (e.g. 'u_boot_spl') + * @_prop_name: Property value to get from that entry (e.g. 'pos') + */ +#define binman_sym_declare_optional(_type, _entry_name, _prop_name) \ + _type binman_symname(_entry_name, _prop_name) \ + __attribute__((aligned(4), weak, unused, \ + section(".binman_sym"))) + +/** + * binman_sym() - Access a previously declared symbol + * + * This is used to get the value of a symbol. E.g.: + * + * ulong address = binman_sym(ulong, u_boot_spl, pos); + * + * @_type: Type f the symbol (e.g. unsigned long) + * @entry_name: Name of the entry to look for (e.g. 'u_boot_spl') + * @_prop_name: Property value to get from that entry (e.g. 'pos') + * @returns value of that property (filled in by binman) + */ +#define binman_sym(_type, _entry_name, _prop_name) \ + (*(_type *)&binman_symname(_entry_name, _prop_name)) + +#else /* !BINMAN */ + +#define binman_sym_declare(_type, _entry_name, _prop_name) + +#define binman_sym_declare_optional(_type, _entry_name, _prop_name) + +#define binman_sym(_type, _entry_name, _prop_name) BINMAN_SYM_MISSING + +#endif /* BINMAN */ + +#endif diff --git a/tools/binman/binman.py b/tools/binman/binman.py index aa5139626634..1c8e8dbff65a 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -37,6 +37,7 @@ def RunTests(debug): import entry_test import fdt_test import ftest + import image_test import test import doctest @@ -53,7 +54,8 @@ def RunTests(debug): # 'entry' module. suite = unittest.TestLoader().loadTestsFromTestCase(entry_test.TestEntry) suite.run(result) - for module in (ftest.TestFunctional, fdt_test.TestFdt, elf_test.TestElf): + for module in (ftest.TestFunctional, fdt_test.TestFdt, elf_test.TestElf, + image_test.TestImage): suite = unittest.TestLoader().loadTestsFromTestCase(module) suite.run(result) diff --git a/tools/binman/control.py b/tools/binman/control.py index e175e8d41bd0..ffa2bbd80f69 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -90,8 +90,7 @@ def Binman(options, args): try: tout.Init(options.verbosity) - if options.debug: - elf.debug = True + elf.debug = options.debug try: tools.SetInputDirs(options.indir) tools.PrepareOutputDir(options.outdir, options.preserve) @@ -112,6 +111,7 @@ def Binman(options, args): image.CheckSize() image.CheckEntries() image.ProcessEntryContents() + image.WriteSymbols() image.BuildImage() finally: tools.FinaliseOutputDir() diff --git a/tools/binman/elf.py b/tools/binman/elf.py index 0fb5a4a8ed63..80ff2253f039 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -19,9 +19,6 @@ Symbol = namedtuple('Symbol', ['section', 'address', 'size', 'weak']) -# Used for tests which don't have an ELF file to read -ignore_missing_files = False - def GetSymbols(fname, patterns): """Get the symbols from an ELF file @@ -78,3 +75,55 @@ def GetSymbolAddress(fname, sym_name): if not sym: return None return sym.address + +def LookupAndWriteSymbols(elf_fname, entry, image): + """Replace all symbols in an entry with their correct values + + The entry contents is updated so that values for referenced symbols will be + visible at run time. This is done by finding out the symbols positions in + the entry (using the ELF file) and replacing them with values from binman's + data structures. + + Args: + elf_fname: Filename of ELF image containing the symbol information for + entry + entry: Entry to process + image: Image which can be used to lookup symbol values + """ + fname = tools.GetInputFilename(elf_fname) + syms = GetSymbols(fname, ['image', 'binman']) + if not syms: + return + base = syms.get('__image_copy_start') + if not base: + return + for name, sym in syms.iteritems(): + if name.startswith('_binman'): + msg = ("Image '%s': Symbol '%s'\n in entry '%s'" % + (image.GetPath(), name, entry.GetPath())) + offset = sym.address - base.address + if offset < 0 or offset + sym.size > entry.contents_size: + raise ValueError('%s has offset %x (size %x) but the contents ' + 'size is %x' % (entry.GetPath(), offset, + sym.size, entry.contents_size)) + if sym.size == 4: + pack_string = ' 0) + + if __name__ == '__main__': unittest.main() diff --git a/tools/binman/etype/entry.py b/tools/binman/etype/entry.py index 67c57341ca21..5541887d475c 100644 --- a/tools/binman/etype/entry.py +++ b/tools/binman/etype/entry.py @@ -198,3 +198,11 @@ def SetPositionSize(self, pos, size): def ProcessContents(self): pass + + def WriteSymbols(self, image): + """Write symbol values into binary files for access at run time + + Args: + image: Image containing the entry + """ + pass diff --git a/tools/binman/etype/u_boot_spl.py b/tools/binman/etype/u_boot_spl.py index 68b0148427dc..3720b47fef63 100644 --- a/tools/binman/etype/u_boot_spl.py +++ b/tools/binman/etype/u_boot_spl.py @@ -6,12 +6,18 @@ # Entry-type module for spl/u-boot-spl.bin # +import elf + from entry import Entry from blob import Entry_blob class Entry_u_boot_spl(Entry_blob): def __init__(self, image, etype, node): Entry_blob.__init__(self, image, etype, node) + self.elf_fname = 'spl/u-boot-spl' def GetDefaultFilename(self): return 'spl/u-boot-spl.bin' + + def WriteSymbols(self, image): + elf.LookupAndWriteSymbols(self.elf_fname, self, image) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 2bee6a168f38..5812ab397cf3 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -20,6 +20,7 @@ import cmdline import command import control +import elf import fdt import fdt_util import tools @@ -573,6 +574,8 @@ def testPackAlignPowerOf2(self): def testImagePadByte(self): """Test that the image pad byte can be specified""" + with open(self.TestFile('bss_data')) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) data = self._DoReadFile('21_image_pad.dts') self.assertEqual(U_BOOT_SPL_DATA + (chr(0xff) * 1) + U_BOOT_DATA, data) @@ -889,6 +892,22 @@ def testSplNoDtb(self): data = self._DoReadFile('52_u_boot_spl_nodtb.dts') self.assertEqual(U_BOOT_SPL_NODTB_DATA, data[:len(U_BOOT_SPL_NODTB_DATA)]) + def testSymbols(self): + """Test binman can assign symbols embedded in U-Boot""" + elf_fname = self.TestFile('u_boot_binman_syms') + syms = elf.GetSymbols(elf_fname, ['binman', 'image']) + addr = elf.GetSymbolAddress(elf_fname, '__image_copy_start') + self.assertEqual(syms['_binman_u_boot_spl_prop_pos'].address, addr) + + with open(self.TestFile('u_boot_binman_syms')) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + data = self._DoReadFile('53_symbols.dts') + sym_values = struct.pack('_prop_ where is the name of + the entry and is the property to find (e.g. + _binman_u_boot_prop_pos). As a special case, you can append + _any to to have it search for any matching entry. E.g. + _binman_u_boot_any_prop_pos will match entries called u-boot, + u-boot-img and u-boot-nodtb) + optional: True if the symbol is optional. If False this function + will raise if the symbol is not found + msg: Message to display if an error occurs + + Returns: + Value that should be assigned to that symbol, or None if it was + optional and not found + + Raises: + ValueError if the symbol is invalid or not found, or references a + property which is not supported + """ + m = re.match(r'^_binman_(\w+)_prop_(\w+)$', sym_name) + if not m: + raise ValueError("%s: Symbol '%s' has invalid format" % + (msg, sym_name)) + entry_name, prop_name = m.groups() + entry_name = entry_name.replace('_', '-') + entry = self._entries.get(entry_name) + if not entry: + if entry_name.endswith('-any'): + root = entry_name[:-4] + for name in self._entries: + if name.startswith(root): + rest = name[len(root):] + if rest in ['', '-img', '-nodtb']: + entry = self._entries[name] + if not entry: + err = ("%s: Entry '%s' not found in list (%s)" % + (msg, entry_name, ','.join(self._entries.keys()))) + if optional: + print('Warning: %s' % err, file=sys.stderr) + return None + raise ValueError(err) + if prop_name == 'pos': + return entry.pos + else: + raise ValueError("%s: No such property '%s'" % (msg, prop_name)) diff --git a/tools/binman/image_test.py b/tools/binman/image_test.py new file mode 100644 index 000000000000..1b50dda4dca9 --- /dev/null +++ b/tools/binman/image_test.py @@ -0,0 +1,46 @@ +# +# Copyright (c) 2017 Google, Inc +# Written by Simon Glass +# +# SPDX-License-Identifier: GPL-2.0+ +# +# Test for the image module + +import unittest + +from image import Image +from elf_test import capture_sys_output + +class TestImage(unittest.TestCase): + def testInvalidFormat(self): + image = Image('name', 'node', test=True) + with self.assertRaises(ValueError) as e: + image.LookupSymbol('_binman_something_prop_', False, 'msg') + self.assertIn( + "msg: Symbol '_binman_something_prop_' has invalid format", + str(e.exception)) + + def testMissingSymbol(self): + image = Image('name', 'node', test=True) + image._entries = {} + with self.assertRaises(ValueError) as e: + image.LookupSymbol('_binman_type_prop_pname', False, 'msg') + self.assertIn("msg: Entry 'type' not found in list ()", + str(e.exception)) + + def testMissingSymbolOptional(self): + image = Image('name', 'node', test=True) + image._entries = {} + with capture_sys_output() as (stdout, stderr): + val = image.LookupSymbol('_binman_type_prop_pname', True, 'msg') + self.assertEqual(val, None) + self.assertEqual("Warning: msg: Entry 'type' not found in list ()\n", + stderr.getvalue()) + self.assertEqual('', stdout.getvalue()) + + def testBadProperty(self): + image = Image('name', 'node', test=True) + image._entries = {'u-boot': 1} + with self.assertRaises(ValueError) as e: + image.LookupSymbol('_binman_u_boot_prop_bad', False, 'msg') + self.assertIn("msg: No such property 'bad", str(e.exception)) diff --git a/tools/binman/test/53_symbols.dts b/tools/binman/test/53_symbols.dts new file mode 100644 index 000000000000..980b066eb21d --- /dev/null +++ b/tools/binman/test/53_symbols.dts @@ -0,0 +1,20 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0xff>; + u-boot-spl { + }; + + u-boot { + pos = <20>; + }; + + u-boot-spl2 { + type = "u-boot-spl"; + }; + }; +}; From cf2a8fd66d8d4b855f5955e15e4d8e436b4bc3d5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:55:02 -0700 Subject: [PATCH 09/17] binman: arm: Include the binman symbol table This area of the image contains symbols whose values are filled in by binman. If this feature is not used, the table is empty. Add this to the ARM SPL link script. Signed-off-by: Simon Glass --- arch/arm/config.mk | 6 ++++-- arch/arm/cpu/u-boot-spl.lds | 7 +++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 02f61fcc3cba..9c213b897cd5 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -136,10 +136,12 @@ endif # limit ourselves to the sections we want in the .bin. ifdef CONFIG_ARM64 OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j .rodata -j .data \ - -j .u_boot_list -j .rela.dyn -j .got -j .got.plt + -j .u_boot_list -j .rela.dyn -j .got -j .got.plt \ + -j .binman_sym_table else OBJCOPYFLAGS += -j .text -j .secure_text -j .secure_data -j .rodata -j .hash \ - -j .data -j .got -j .got.plt -j .u_boot_list -j .rel.dyn + -j .data -j .got -j .got.plt -j .u_boot_list -j .rel.dyn \ + -j .binman_sym_table endif # if a dtb section exists we always have to include it diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index 068163b73a65..65f7b68861e2 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -36,6 +36,13 @@ SECTIONS KEEP(*(SORT(.u_boot_list*))); } + . = ALIGN(4); + .binman_sym_table : { + __binman_sym_start = .; + KEEP(*(SORT(.binman_sym*))); + __binman_sym_end = .; + } + . = ALIGN(4); __image_copy_end = .; From 8bee2d251afb61c203aa94877cf5077731822ed5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:55:03 -0700 Subject: [PATCH 10/17] binman: Add binman symbol support to SPL Allow SPL to access binman symbols and use this to get the address of U-Boot. This falls back to CONFIG_SYS_TEXT_BASE if the binman symbol is not available. Signed-off-by: Simon Glass --- common/spl/spl.c | 16 ++++++++++++++-- include/binman_sym.h | 13 +++++++++++++ include/spl.h | 11 +++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/common/spl/spl.c b/common/spl/spl.c index 3bb20c7822f8..76c1963611c4 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -8,6 +8,7 @@ */ #include +#include #include #include #include @@ -32,6 +33,9 @@ DECLARE_GLOBAL_DATA_PTR; u32 *boot_params_ptr = NULL; +/* See spl.h for information about this */ +binman_sym_declare(ulong, u_boot_any, pos); + /* Define board data structure */ static bd_t bdata __attribute__ ((section(".data"))); @@ -120,9 +124,17 @@ __weak void spl_board_prepare_for_boot(void) void spl_set_header_raw_uboot(struct spl_image_info *spl_image) { + ulong u_boot_pos = binman_sym(ulong, u_boot_any, pos); + spl_image->size = CONFIG_SYS_MONITOR_LEN; - spl_image->entry_point = CONFIG_SYS_UBOOT_START; - spl_image->load_addr = CONFIG_SYS_TEXT_BASE; + if (u_boot_pos != BINMAN_SYM_MISSING) { + /* biman does not support separate entry addresses at present */ + spl_image->entry_point = u_boot_pos; + spl_image->load_addr = u_boot_pos; + } else { + spl_image->entry_point = CONFIG_SYS_UBOOT_START; + spl_image->load_addr = CONFIG_SYS_TEXT_BASE; + } spl_image->os = IH_OS_U_BOOT; spl_image->name = "U-Boot"; } diff --git a/include/binman_sym.h b/include/binman_sym.h index 3999b26d8d4f..87d03d529452 100644 --- a/include/binman_sym.h +++ b/include/binman_sym.h @@ -37,6 +37,17 @@ _type binman_symname(_entry_name, _prop_name) \ __attribute__((aligned(4), unused, section(".binman_sym"))) +/** + * binman_sym_extern() - Declare a extern symbol that will be used at run-time + * + * @_type: Type f the symbol (e.g. unsigned long) + * @entry_name: Name of the entry to look for (e.g. 'u_boot_spl') + * @_prop_name: Property value to get from that entry (e.g. 'pos') + */ +#define binman_sym_extern(_type, _entry_name, _prop_name) \ + extern _type binman_symname(_entry_name, _prop_name) \ + __attribute__((aligned(4), unused, section(".binman_sym"))) + /** * binman_sym_declare_optional() - Declare an optional symbol * @@ -73,6 +84,8 @@ #define binman_sym_declare_optional(_type, _entry_name, _prop_name) +#define binman_sym_extern(_type, _entry_name, _prop_name) + #define binman_sym(_type, _entry_name, _prop_name) BINMAN_SYM_MISSING #endif /* BINMAN */ diff --git a/include/spl.h b/include/spl.h index 308ce7b563bf..c14448b8fc98 100644 --- a/include/spl.h +++ b/include/spl.h @@ -7,6 +7,8 @@ #ifndef _SPL_H_ #define _SPL_H_ +#include + /* Platform-specific defines */ #include #include @@ -51,6 +53,15 @@ struct spl_load_info { void *buf); }; +/* + * We need to know the position of U-Boot in memory so we can jump to it. We + * allow any U-Boot binary to be used (u-boot.bin, u-boot-nodtb.bin, + * u-boot.img), hence the '_any'. These is no checking here that the correct + * image is found. For * example if u-boot.img is used we don't check that + * spl_parse_image_header() can parse a valid header. + */ +binman_sym_extern(ulong, u_boot_any, pos); + /** * spl_load_simple_fit() - Loads a fit image from a device. * @spl_image: Image description to set up From dfce1799e71fd6f36efcea2867b668ab5665e7e1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:55:04 -0700 Subject: [PATCH 11/17] binman: Add binman support to spl_ram.c SPL supports reading U-Boot from a RAM location. At present this is hard-coded to the U-Boot text base address. Use binman to allow this to come from the image file, if binman is used. Signed-off-by: Simon Glass --- common/spl/spl_ram.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/common/spl/spl_ram.c b/common/spl/spl_ram.c index b2645a194806..fa8c768773af 100644 --- a/common/spl/spl_ram.c +++ b/common/spl/spl_ram.c @@ -11,6 +11,8 @@ * SPDX-License-Identifier: GPL-2.0+ */ #include +#include +#include #include #include @@ -48,15 +50,24 @@ static int spl_ram_load_image(struct spl_image_info *spl_image, load.read = spl_ram_load_read; spl_load_simple_fit(spl_image, &load, 0, header); } else { + ulong u_boot_pos = binman_sym(ulong, u_boot_any, pos); + debug("Legacy image\n"); /* * Get the header. It will point to an address defined by * handoff which will tell where the image located inside - * the flash. For now, it will temporary fixed to address - * pointed by U-Boot. + * the flash. */ - header = (struct image_header *) - (CONFIG_SYS_TEXT_BASE - sizeof(struct image_header)); + debug("u_boot_pos = %lx\n", u_boot_pos); + if (u_boot_pos == BINMAN_SYM_MISSING) { + /* + * No binman support or no information. For now, fix it + * to the address pointed to by U-Boot. + */ + u_boot_pos = CONFIG_SYS_TEXT_BASE - + sizeof(struct image_header); + } + header = (struct image_header *)map_sysmem(u_boot_pos, 0); spl_parse_image_header(spl_image, header); } From 39c1502ccc49cc5e9ef799ca56c6fa66c8863504 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:55:05 -0700 Subject: [PATCH 12/17] binman: Add documentation for the symbol feature Add this feature to the README. Signed-off-by: Simon Glass Reviewed-by: Lukasz Majewski --- tools/binman/README | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tools/binman/README b/tools/binman/README index 4ef76c8f0897..08c3e56bdef8 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -439,6 +439,8 @@ contents of an entry in some way. For example, it would be possible to create an entry containing a hash of the contents of some other entries. At this stage the position and size of entries should not be adjusted. +6. WriteEntryInfo() + 7. BuildImage() - builds the image and writes it to a file. This is the final step. @@ -471,6 +473,33 @@ the 'warning' line in scripts/Makefile.lib to see what it has found: # u_boot_dtsi_options_debug = $(u_boot_dtsi_options_raw) +Access to binman entry positions at run time +-------------------------------------------- + +Binman assembles images and determines where each entry is placed in the image. +This information may be useful to U-Boot at run time. For example, in SPL it +is useful to be able to find the location of U-Boot so that it can be executed +when SPL is finished. + +Binman allows you to declare symbols in the SPL image which are filled in +with their correct values during the build. For example: + + binman_sym_declare(ulong, u_boot_any, pos); + +declares a ulong value which will be assigned to the position of any U-Boot +image (u-boot.bin, u-boot.img, u-boot-nodtb.bin) that is present in the image. +You can access this value with something like: + + ulong u_boot_pos = binman_sym(ulong, u_boot_any, pos); + +Thus u_boot_pos will be set to the position of U-Boot in memory, assuming that +the whole image has been loaded, or is available in flash. You can then jump to +that address to start U-Boot. + +At present this feature is only supported in SPL. In principle it is possible +to fill in such symbols in U-Boot proper, as well. + + Code coverage ------------- @@ -543,7 +572,8 @@ To do Some ideas: - Fill out the device tree to include the final position and size of each - entry (since the input file may not always specify these) + entry (since the input file may not always specify these). See also + 'Access to binman entry positions at run time' above - Use of-platdata to make the information available to code that is unable to use device tree (such as a very small SPL image) - Write an image map to a text file From f2faffecb016988a999f26f1dbebc5d88793761b Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 13 Nov 2017 18:55:06 -0700 Subject: [PATCH 13/17] binman: tegra: Convert to use binman Update tegra to use binman for image creation. This still includes the current Makefile logic, but a later patch will remove this. Three output files are created, all of which combine SPL and U-Boot: u-boot-tegra.bin - standard image u-boot-dtb-tegra.bin - same as u-boot-tegra.bin u-boot-nodtb-target.bin - includes U-Boot without the appended device tree The latter is useful for build systems where the device is appended later, perhaps after being modified. Signed-off-by: Simon Glass --- Makefile | 6 ++++ arch/arm/dts/tegra-u-boot.dtsi | 40 ++++++++++++++++++++++ arch/arm/dts/tegra114-u-boot.dtsi | 3 ++ arch/arm/dts/tegra124-nyan-big-u-boot.dtsi | 2 ++ arch/arm/dts/tegra124-u-boot.dtsi | 3 ++ arch/arm/dts/tegra20-u-boot.dtsi | 11 ++---- arch/arm/dts/tegra210-u-boot.dtsi | 3 ++ arch/arm/dts/tegra30-u-boot.dtsi | 3 ++ arch/arm/mach-tegra/Kconfig | 1 + 9 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 arch/arm/dts/tegra-u-boot.dtsi create mode 100644 arch/arm/dts/tegra114-u-boot.dtsi create mode 100644 arch/arm/dts/tegra124-u-boot.dtsi create mode 100644 arch/arm/dts/tegra210-u-boot.dtsi create mode 100644 arch/arm/dts/tegra30-u-boot.dtsi diff --git a/Makefile b/Makefile index e6d309afe42c..9800d79c8598 100644 --- a/Makefile +++ b/Makefile @@ -1149,6 +1149,11 @@ u-boot-sunxi-with-spl.bin: spl/sunxi-spl.bin u-boot.img u-boot.dtb FORCE endif ifneq ($(CONFIG_TEGRA),) +ifneq ($(CONFIG_BINMAN),) +u-boot-dtb-tegra.bin u-boot-tegra.bin u-boot-nodtb-tegra.bin: \ + spl/u-boot-spl u-boot.bin FORCE + $(call if_changed,binman) +else OBJCOPYFLAGS_u-boot-nodtb-tegra.bin = -O binary --pad-to=$(CONFIG_SYS_TEXT_BASE) u-boot-nodtb-tegra.bin: spl/u-boot-spl u-boot-nodtb.bin FORCE $(call if_changed,pad_cat) @@ -1159,6 +1164,7 @@ u-boot-tegra.bin: spl/u-boot-spl u-boot.bin FORCE u-boot-dtb-tegra.bin: u-boot-tegra.bin FORCE $(call if_changed,copy) +endif # binman endif OBJCOPYFLAGS_u-boot-app.efi := $(OBJCOPYFLAGS_EFI) diff --git a/arch/arm/dts/tegra-u-boot.dtsi b/arch/arm/dts/tegra-u-boot.dtsi new file mode 100644 index 000000000000..cde591c5fca4 --- /dev/null +++ b/arch/arm/dts/tegra-u-boot.dtsi @@ -0,0 +1,40 @@ +#include + +/ { + binman { + multiple-images; + image1 { + filename = "u-boot-tegra.bin"; + pad-byte = <0xff>; + u-boot-spl { + }; + u-boot { + pos = <(CONFIG_SYS_TEXT_BASE - + CONFIG_SPL_TEXT_BASE)>; + }; + }; + + /* Same as image1 - some tools still expect the -dtb suffix */ + image2 { + filename = "u-boot-dtb-tegra.bin"; + pad-byte = <0xff>; + u-boot-spl { + }; + u-boot { + pos = <(CONFIG_SYS_TEXT_BASE - + CONFIG_SPL_TEXT_BASE)>; + }; + }; + + image3 { + filename = "u-boot-nodtb-tegra.bin"; + pad-byte = <0xff>; + u-boot-spl { + }; + u-boot-nodtb { + pos = <(CONFIG_SYS_TEXT_BASE - + CONFIG_SPL_TEXT_BASE)>; + }; + }; + }; +}; diff --git a/arch/arm/dts/tegra114-u-boot.dtsi b/arch/arm/dts/tegra114-u-boot.dtsi new file mode 100644 index 000000000000..7c1197255284 --- /dev/null +++ b/arch/arm/dts/tegra114-u-boot.dtsi @@ -0,0 +1,3 @@ +#include + +#include "tegra-u-boot.dtsi" diff --git a/arch/arm/dts/tegra124-nyan-big-u-boot.dtsi b/arch/arm/dts/tegra124-nyan-big-u-boot.dtsi index 65c3851affac..44e64998c5f0 100644 --- a/arch/arm/dts/tegra124-nyan-big-u-boot.dtsi +++ b/arch/arm/dts/tegra124-nyan-big-u-boot.dtsi @@ -5,6 +5,8 @@ * SPDX-License-Identifier: GPL-2.0+ */ +#include "tegra-u-boot.dtsi" + / { host1x@50000000 { u-boot,dm-pre-reloc; diff --git a/arch/arm/dts/tegra124-u-boot.dtsi b/arch/arm/dts/tegra124-u-boot.dtsi new file mode 100644 index 000000000000..7c1197255284 --- /dev/null +++ b/arch/arm/dts/tegra124-u-boot.dtsi @@ -0,0 +1,3 @@ +#include + +#include "tegra-u-boot.dtsi" diff --git a/arch/arm/dts/tegra20-u-boot.dtsi b/arch/arm/dts/tegra20-u-boot.dtsi index 9b9835da7e96..7c1197255284 100644 --- a/arch/arm/dts/tegra20-u-boot.dtsi +++ b/arch/arm/dts/tegra20-u-boot.dtsi @@ -1,8 +1,3 @@ -/ { - host1x@50000000 { - u-boot,dm-pre-reloc; - dc@54200000 { - u-boot,dm-pre-reloc; - }; - }; -}; +#include + +#include "tegra-u-boot.dtsi" diff --git a/arch/arm/dts/tegra210-u-boot.dtsi b/arch/arm/dts/tegra210-u-boot.dtsi new file mode 100644 index 000000000000..7c1197255284 --- /dev/null +++ b/arch/arm/dts/tegra210-u-boot.dtsi @@ -0,0 +1,3 @@ +#include + +#include "tegra-u-boot.dtsi" diff --git a/arch/arm/dts/tegra30-u-boot.dtsi b/arch/arm/dts/tegra30-u-boot.dtsi new file mode 100644 index 000000000000..7c1197255284 --- /dev/null +++ b/arch/arm/dts/tegra30-u-boot.dtsi @@ -0,0 +1,3 @@ +#include + +#include "tegra-u-boot.dtsi" diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig index 51e50907d27a..51d143687b06 100644 --- a/arch/arm/mach-tegra/Kconfig +++ b/arch/arm/mach-tegra/Kconfig @@ -38,6 +38,7 @@ config TEGRA_COMMON select OF_CONTROL select VIDCONSOLE_AS_LCD if DM_VIDEO select BOARD_EARLY_INIT_F + select BINMAN imply CRC32_VERIFY config TEGRA_NO_BPMP From b53f6992e9cb7f0d892ebc2b1620b55559f461be Mon Sep 17 00:00:00 2001 From: Philipp Tomsich Date: Fri, 24 Nov 2017 18:37:58 +0100 Subject: [PATCH 14/17] dm: reset: have the reset-command perform a COLD reset The DM version of do_reset has been issuing a warm-reset, which (on some platforms keeps GPIOs and other parts of the platform active). This may cause unintended behaviour, as calling do_reset usually indicates a desire to reset the board/platform and not just the CPU. This changes do_reset to always request a COLD reset. Note that programmatic uses can still invoke a WARM reset through reset_cpu() or using sysreset_walk(). Signed-off-by: Philipp Tomsich Reviewed-by: Simon Glass --- drivers/sysreset/sysreset-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/sysreset/sysreset-uclass.c b/drivers/sysreset/sysreset-uclass.c index 3566d17fb1b7..0747c52b6077 100644 --- a/drivers/sysreset/sysreset-uclass.c +++ b/drivers/sysreset/sysreset-uclass.c @@ -70,7 +70,7 @@ void reset_cpu(ulong addr) int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { - sysreset_walk_halt(SYSRESET_WARM); + sysreset_walk_halt(SYSRESET_COLD); return 0; } From bf802f5d544f85c03b4097ab23d078be43c61855 Mon Sep 17 00:00:00 2001 From: Felix Brack Date: Mon, 27 Nov 2017 09:14:16 +0100 Subject: [PATCH 15/17] power: extend prefix match to regulator-name property This patch extends pmic_bind_children prefix matching. In addition to the node name the property regulator-name is used while trying to match prefixes. This allows assigning different drivers to regulator nodes named regulator@1 and regulator@10 for example. I have discarded the idea of using other properties then regulator-name as I do not see any benefit in using property compatible or even regulator-compatible. Of course I am open to change this if there are good reasons to do so. Signed-off-by: Felix Brack Reviewed-by: Simon Glass --- arch/sandbox/dts/sandbox_pmic.dtsi | 6 ++++++ doc/device-tree-bindings/regulator/regulator.txt | 16 ++++++++++++++-- drivers/power/pmic/pmic-uclass.c | 11 +++++++++-- include/power/sandbox_pmic.h | 5 ++++- test/dm/regulator.c | 2 ++ 5 files changed, 35 insertions(+), 5 deletions(-) diff --git a/arch/sandbox/dts/sandbox_pmic.dtsi b/arch/sandbox/dts/sandbox_pmic.dtsi index ce261b930e13..acb479939645 100644 --- a/arch/sandbox/dts/sandbox_pmic.dtsi +++ b/arch/sandbox/dts/sandbox_pmic.dtsi @@ -75,4 +75,10 @@ regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; }; + + no_match_by_nodename { + regulator-name = "buck_SUPPLY_1.5V"; + regulator-min-microvolt = <1500000>; + regulator-max-microvolt = <1500000>; + }; }; diff --git a/doc/device-tree-bindings/regulator/regulator.txt b/doc/device-tree-bindings/regulator/regulator.txt index 918711eb4d86..65b69c427899 100644 --- a/doc/device-tree-bindings/regulator/regulator.txt +++ b/doc/device-tree-bindings/regulator/regulator.txt @@ -2,7 +2,8 @@ Voltage/Current regulator Binding: The regulator devices don't use the "compatible" property. The binding is done -by the prefix of regulator node's name. Usually the pmic I/O driver will provide +by the prefix of regulator node's name, or, if this fails, by the prefix of the +regulator's "regulator-name" property. Usually the pmic I/O driver will provide the array of 'struct pmic_child_info' with the prefixes and compatible drivers. The bind is done by calling function: pmic_bind_childs(). Example drivers: @@ -15,8 +16,19 @@ For the node name e.g.: "prefix[:alpha:]num { ... }": Example the prefix "ldo" will pass for: "ldo1", "ldo@1", "ldoreg@1, ... +Binding by means of the node's name is preferred. However if the node names +would produce ambiguous prefixes (like "regulator@1" and "regualtor@11") and you +can't or do not want to change them then binding against the "regulator-name" +property is possible. The syntax for the prefix of the "regulator-name" property +is the same as the one for the regulator's node name. +Use case: a regulator named "regulator@1" to be bound to a driver named +"LDO_DRV" and a regulator named "regualator@11" to be bound to an other driver +named "BOOST_DRV". Using prefix "regualtor@1" for driver matching would load +the same driver for both regulators, hence the prefix is ambiguous. + Optional properties: -- regulator-name: a string, required by the regulator uclass +- regulator-name: a string, required by the regulator uclass, used for driver + binding if binding by node's name prefix fails - regulator-min-microvolt: a minimum allowed Voltage value - regulator-max-microvolt: a maximum allowed Voltage value - regulator-min-microamp: a minimum allowed Current value diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c index 64964e4e963d..9347b4068897 100644 --- a/drivers/power/pmic/pmic-uclass.c +++ b/drivers/power/pmic/pmic-uclass.c @@ -26,6 +26,7 @@ int pmic_bind_children(struct udevice *pmic, ofnode parent, struct driver *drv; struct udevice *child; const char *node_name; + const char *reg_name; int bind_count = 0; ofnode node; int prefix_len; @@ -44,8 +45,14 @@ int pmic_bind_children(struct udevice *pmic, ofnode parent, debug(" - compatible prefix: '%s'\n", info->prefix); prefix_len = strlen(info->prefix); - if (strncmp(info->prefix, node_name, prefix_len)) - continue; + if (strncmp(info->prefix, node_name, prefix_len)) { + reg_name = ofnode_read_string(node, + "regulator-name"); + if (!reg_name) + continue; + if (strncmp(info->prefix, reg_name, prefix_len)) + continue; + } drv = lists_driver_lookup_name(info->driver); if (!drv) { diff --git a/include/power/sandbox_pmic.h b/include/power/sandbox_pmic.h index 7fdbfb9fc6f9..c5e6fda2ea0f 100644 --- a/include/power/sandbox_pmic.h +++ b/include/power/sandbox_pmic.h @@ -13,7 +13,7 @@ #define SANDBOX_BUCK_DRIVER "sandbox_buck" #define SANDBOX_OF_BUCK_PREFIX "buck" -#define SANDBOX_BUCK_COUNT 2 +#define SANDBOX_BUCK_COUNT 3 #define SANDBOX_LDO_COUNT 2 /* * Sandbox PMIC registers: @@ -109,6 +109,9 @@ enum { #define SANDBOX_BUCK1_PLATNAME "SUPPLY_1.2V" #define SANDBOX_BUCK2_DEVNAME "buck2" #define SANDBOX_BUCK2_PLATNAME "SUPPLY_3.3V" +/* BUCK3: for testing fallback regulator prefix matching during bind */ +#define SANDBOX_BUCK3_DEVNAME "no_match_by_nodename" +#define SANDBOX_BUCK3_PLATNAME "buck_SUPPLY_1.5V" /* LDO names */ #define SANDBOX_LDO1_DEVNAME "ldo1" #define SANDBOX_LDO1_PLATNAME "VDD_EMMC_1.8V" diff --git a/test/dm/regulator.c b/test/dm/regulator.c index 3d0056f2dc7b..395381d4bd2f 100644 --- a/test/dm/regulator.c +++ b/test/dm/regulator.c @@ -27,6 +27,7 @@ DECLARE_GLOBAL_DATA_PTR; enum { BUCK1, BUCK2, + BUCK3, LDO1, LDO2, OUTPUT_COUNT, @@ -42,6 +43,7 @@ static const char *regulator_names[OUTPUT_COUNT][OUTPUT_NAME_COUNT] = { /* devname, platname */ { SANDBOX_BUCK1_DEVNAME, SANDBOX_BUCK1_PLATNAME }, { SANDBOX_BUCK2_DEVNAME, SANDBOX_BUCK2_PLATNAME }, + { SANDBOX_BUCK3_DEVNAME, SANDBOX_BUCK3_PLATNAME }, { SANDBOX_LDO1_DEVNAME, SANDBOX_LDO1_PLATNAME}, { SANDBOX_LDO2_DEVNAME, SANDBOX_LDO2_PLATNAME}, }; From 8a5cbc065dfe1f258e3a7be823ea128184b90b5b Mon Sep 17 00:00:00 2001 From: Stefan Roese Date: Wed, 29 Nov 2017 16:46:42 +0100 Subject: [PATCH 16/17] dm: blk: Use uclass_find_first/next_device() in blk_first/next_device() This patch changes the calls to uclass_first/next_device() in blk_first/ next_device() to use uclass_find_first/next_device() instead. These functions don't prepare the devices, which is correct in this case. With this patch applied, the "usb storage" command now works again as expected: => usb storage Device 0: Vendor: SanDisk Rev: 1.00 Prod: Ultra Type: Removable Hard Disk Capacity: 58656.0 MB = 57.2 GB (120127488 x 512) Without this patch, it used to generate this buggy output: => usb storage Card did not respond to voltage select! mmc_init: -95, time 26 No storage devices, perhaps not 'usb start'ed..? Signed-off-by: Stefan Roese Suggested-by: Simon Glass Cc: Simon Glass Cc: Bin Meng Reviewed-by: Bin Meng Reviewed-by: Simon Glass --- drivers/block/blk-uclass.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 537cf5f0bbcb..010ed32d3add 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -10,6 +10,7 @@ #include #include #include +#include static const char *if_typename_str[IF_TYPE_COUNT] = { [IF_TYPE_IDE] = "ide", @@ -331,7 +332,7 @@ int blk_first_device(int if_type, struct udevice **devp) struct blk_desc *desc; int ret; - ret = uclass_first_device(UCLASS_BLK, devp); + ret = uclass_find_first_device(UCLASS_BLK, devp); if (ret) return ret; if (!*devp) @@ -340,7 +341,7 @@ int blk_first_device(int if_type, struct udevice **devp) desc = dev_get_uclass_platdata(*devp); if (desc->if_type == if_type) return 0; - ret = uclass_next_device(devp); + ret = uclass_find_next_device(devp); if (ret) return ret; } while (*devp); @@ -356,7 +357,7 @@ int blk_next_device(struct udevice **devp) desc = dev_get_uclass_platdata(*devp); if_type = desc->if_type; do { - ret = uclass_next_device(devp); + ret = uclass_find_next_device(devp); if (ret) return ret; if (!*devp) From 854dfbf99b89c114ba100905e1500b8ace60e0f9 Mon Sep 17 00:00:00 2001 From: Felix Brack Date: Thu, 30 Nov 2017 13:52:37 +0100 Subject: [PATCH 17/17] power: pmic/regulator: Add basic support for TPS65910 Texas Instrument's TPS65910 PMIC contains 3 buck DC-DC converts, one boost DC-DC converter and 8 LDOs. This patch implements driver model support for the TPS65910 PMIC and its regulators making the get/set API for regulator value/enable available. This patch depends on the patch "am33xx: Add a function to query MPU voltage in uV" to build correctly. For boards relying on the DT include file tps65910.dtsi the v3 patch "power: extend prefix match to regulator-name property" and an appropriate regulator naming is also required. Signed-off-by: Felix Brack Reviewed-by: Simon Glass --- drivers/power/pmic/Kconfig | 8 + drivers/power/pmic/Makefile | 1 + drivers/power/pmic/pmic_tps65910_dm.c | 98 ++++ drivers/power/regulator/Kconfig | 8 + drivers/power/regulator/Makefile | 1 + drivers/power/regulator/tps65910_regulator.c | 459 +++++++++++++++++++ include/power/tps65910_pmic.h | 130 ++++++ 7 files changed, 705 insertions(+) create mode 100644 drivers/power/pmic/pmic_tps65910_dm.c create mode 100644 drivers/power/regulator/tps65910_regulator.c create mode 100644 include/power/tps65910_pmic.h diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig index e3f9e4dfc0de..5d49c93f32a2 100644 --- a/drivers/power/pmic/Kconfig +++ b/drivers/power/pmic/Kconfig @@ -201,3 +201,11 @@ config POWER_MC34VR500 The MC34VR500 is used in conjunction with the FSL T1 and LS1 series SoC. It provides 4 buck DC-DC convertors and 5 LDOs, and it is accessed via an I2C interface. + +config DM_PMIC_TPS65910 + bool "Enable driver for Texas Instruments TPS65910 PMIC" + depends on DM_PMIC + ---help--- + The TPS65910 is a PMIC containing 3 buck DC-DC converters, one boost + DC-DC converter, 8 LDOs and a RTC. This driver binds the SMPS and LDO + pmic children. diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile index f7bdfa560963..7d6c583d347c 100644 --- a/drivers/power/pmic/Makefile +++ b/drivers/power/pmic/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_PMIC_RK8XX) += rk8xx.o obj-$(CONFIG_PMIC_RN5T567) += rn5t567.o obj-$(CONFIG_PMIC_TPS65090) += tps65090.o obj-$(CONFIG_PMIC_S5M8767) += s5m8767.o +obj-$(CONFIG_DM_PMIC_TPS65910) += pmic_tps65910_dm.o obj-$(CONFIG_$(SPL_)PMIC_PALMAS) += palmas.o obj-$(CONFIG_$(SPL_)PMIC_LP873X) += lp873x.o obj-$(CONFIG_$(SPL_)PMIC_LP87565) += lp87565.o diff --git a/drivers/power/pmic/pmic_tps65910_dm.c b/drivers/power/pmic/pmic_tps65910_dm.c new file mode 100644 index 000000000000..0127ce3aa05c --- /dev/null +++ b/drivers/power/pmic/pmic_tps65910_dm.c @@ -0,0 +1,98 @@ +/* + * Copyright (C) EETS GmbH, 2017, Felix Brack + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include +#include +#include +#include +#include +#include + +DECLARE_GLOBAL_DATA_PTR; + +static const struct pmic_child_info pmic_children_info[] = { + { .prefix = "ldo_", .driver = TPS65910_LDO_DRIVER }, + { .prefix = "buck_", .driver = TPS65910_BUCK_DRIVER }, + { .prefix = "boost_", .driver = TPS65910_BOOST_DRIVER }, + { }, +}; + +static int pmic_tps65910_reg_count(struct udevice *dev) +{ + return TPS65910_NUM_REGS; +} + +static int pmic_tps65910_write(struct udevice *dev, uint reg, const u8 *buffer, + int len) +{ + int ret; + + ret = dm_i2c_write(dev, reg, buffer, len); + if (ret) + error("%s write error on register %02x\n", dev->name, reg); + + return ret; +} + +static int pmic_tps65910_read(struct udevice *dev, uint reg, u8 *buffer, + int len) +{ + int ret; + + ret = dm_i2c_read(dev, reg, buffer, len); + if (ret) + error("%s read error on register %02x\n", dev->name, reg); + + return ret; +} + +static int pmic_tps65910_bind(struct udevice *dev) +{ + ofnode regulators_node; + int children; + + regulators_node = dev_read_subnode(dev, "regulators"); + if (!ofnode_valid(regulators_node)) { + debug("%s regulators subnode not found\n", dev->name); + return -EINVAL; + } + + children = pmic_bind_children(dev, regulators_node, pmic_children_info); + if (!children) + debug("%s has no children (regulators)\n", dev->name); + + return 0; +} + +static int pmic_tps65910_probe(struct udevice *dev) +{ + /* use I2C control interface instead of I2C smartreflex interface to + * access smartrefelex registers VDD1_OP_REG, VDD1_SR_REG, VDD2_OP_REG + * and VDD2_SR_REG + */ + return pmic_clrsetbits(dev, TPS65910_REG_DEVICE_CTRL, 0, + TPS65910_I2C_SEL_MASK); +} + +static struct dm_pmic_ops pmic_tps65910_ops = { + .reg_count = pmic_tps65910_reg_count, + .read = pmic_tps65910_read, + .write = pmic_tps65910_write, +}; + +static const struct udevice_id pmic_tps65910_match[] = { + { .compatible = "ti,tps65910" }, + { /* sentinel */ } +}; + +U_BOOT_DRIVER(pmic_tps65910) = { + .name = "pmic_tps65910", + .id = UCLASS_PMIC, + .of_match = pmic_tps65910_match, + .bind = pmic_tps65910_bind, + .probe = pmic_tps65910_probe, + .ops = &pmic_tps65910_ops, +}; diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig index 8892fa14e022..26fb9368ea9e 100644 --- a/drivers/power/regulator/Kconfig +++ b/drivers/power/regulator/Kconfig @@ -188,3 +188,11 @@ config DM_REGULATOR_LP87565 LP87565 series of PMICs have 4 single phase BUCKs that can also be configured in multi phase modes. The driver implements get/set api for value and enable. + +config DM_REGULATOR_TPS65910 + bool "Enable driver for TPS65910 PMIC regulators" + depends on DM_PMIC_TPS65910 + ---help--- + The TPS65910 PMIC provides 4 SMPSs and 8 LDOs. This driver supports all + regulator types of the TPS65910 (BUCK, BOOST and LDO). It implements + the get/set api for value and enable. diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile index 6c149a926347..7a2e76dc8293 100644 --- a/drivers/power/regulator/Makefile +++ b/drivers/power/regulator/Makefile @@ -21,3 +21,4 @@ obj-$(CONFIG_$(SPL_)DM_REGULATOR_PALMAS) += palmas_regulator.o obj-$(CONFIG_$(SPL_)DM_REGULATOR_PBIAS) += pbias_regulator.o obj-$(CONFIG_$(SPL_)DM_REGULATOR_LP873X) += lp873x_regulator.o obj-$(CONFIG_$(SPL_)DM_REGULATOR_LP87565) += lp87565_regulator.o +obj-$(CONFIG_DM_REGULATOR_TPS65910) += tps65910_regulator.o diff --git a/drivers/power/regulator/tps65910_regulator.c b/drivers/power/regulator/tps65910_regulator.c new file mode 100644 index 000000000000..5e2ec8f36397 --- /dev/null +++ b/drivers/power/regulator/tps65910_regulator.c @@ -0,0 +1,459 @@ +/* + * Copyright (C) EETS GmbH, 2017, Felix Brack + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include +#include +#include +#include +#include + +#define VOUT_CHOICE_COUNT 4 + +/* + * struct regulator_props - Properties of a LDO and VIO SMPS regulator + * + * All of these regulators allow setting one out of four output voltages. + * These output voltages are only achievable when supplying the regulator + * with a minimum input voltage. + * + * @vin_min[]: minimum supply input voltage in uV required to achieve the + * corresponding vout[] voltage + * @vout[]: regulator output voltage in uV + * @reg: I2C register used to set regulator voltage + */ +struct regulator_props { + int vin_min[VOUT_CHOICE_COUNT]; + int vout[VOUT_CHOICE_COUNT]; + int reg; +}; + +static const struct regulator_props ldo_props_vdig1 = { + .vin_min = { 1700000, 2100000, 2700000, 3200000 }, + .vout = { 1200000, 1500000, 1800000, 2700000 }, + .reg = TPS65910_REG_VDIG1 +}; + +static const struct regulator_props ldo_props_vdig2 = { + .vin_min = { 1700000, 1700000, 1700000, 2700000 }, + .vout = { 1000000, 1100000, 1200000, 1800000 }, + .reg = TPS65910_REG_VDIG2 +}; + +static const struct regulator_props ldo_props_vpll = { + .vin_min = { 2700000, 2700000, 2700000, 3000000 }, + .vout = { 1000000, 1100000, 1800000, 2500000 }, + .reg = TPS65910_REG_VPLL +}; + +static const struct regulator_props ldo_props_vdac = { + .vin_min = { 2700000, 3000000, 3200000, 3200000 }, + .vout = { 1800000, 2600000, 2800000, 2850000 }, + .reg = TPS65910_REG_VDAC +}; + +static const struct regulator_props ldo_props_vaux1 = { + .vin_min = { 2700000, 3200000, 3200000, 3200000 }, + .vout = { 1800000, 2500000, 2800000, 2850000 }, + .reg = TPS65910_REG_VAUX1 +}; + +static const struct regulator_props ldo_props_vaux2 = { + .vin_min = { 2700000, 3200000, 3200000, 3600000 }, + .vout = { 1800000, 2800000, 2900000, 3300000 }, + .reg = TPS65910_REG_VAUX2 +}; + +static const struct regulator_props ldo_props_vaux33 = { + .vin_min = { 2700000, 2700000, 3200000, 3600000 }, + .vout = { 1800000, 2000000, 2800000, 3300000 }, + .reg = TPS65910_REG_VAUX33 +}; + +static const struct regulator_props ldo_props_vmmc = { + .vin_min = { 2700000, 3200000, 3200000, 3600000 }, + .vout = { 1800000, 2800000, 3000000, 3300000 }, + .reg = TPS65910_REG_VMMC +}; + +static const struct regulator_props smps_props_vio = { + .vin_min = { 3200000, 3200000, 4000000, 4400000 }, + .vout = { 1500000, 1800000, 2500000, 3300000 }, + .reg = TPS65910_REG_VIO +}; + +/* lookup table of control registers indexed by regulator unit number */ +static const int ctrl_regs[] = { + TPS65910_REG_VRTC, + TPS65910_REG_VIO, + TPS65910_REG_VDD1, + TPS65910_REG_VDD2, + TPS65910_REG_VDD3, + TPS65910_REG_VDIG1, + TPS65910_REG_VDIG2, + TPS65910_REG_VPLL, + TPS65910_REG_VDAC, + TPS65910_REG_VAUX1, + TPS65910_REG_VAUX2, + TPS65910_REG_VAUX33, + TPS65910_REG_VMMC +}; + +/* supply names as used in DT */ +static const char * const supply_names[] = { + "vccio-supply", + "vcc1-supply", + "vcc2-supply", + "vcc3-supply", + "vcc4-supply", + "vcc5-supply", + "vcc6-supply", + "vcc7-supply" +}; + +/* lookup table of regulator supplies indexed by regulator unit number */ +static const int regulator_supplies[] = { + TPS65910_SUPPLY_VCC7, + TPS65910_SUPPLY_VCCIO, + TPS65910_SUPPLY_VCC1, + TPS65910_SUPPLY_VCC2, + TPS65910_SUPPLY_VCC7, + TPS65910_SUPPLY_VCC6, + TPS65910_SUPPLY_VCC6, + TPS65910_SUPPLY_VCC5, + TPS65910_SUPPLY_VCC5, + TPS65910_SUPPLY_VCC4, + TPS65910_SUPPLY_VCC4, + TPS65910_SUPPLY_VCC3, + TPS65910_SUPPLY_VCC3 +}; + +static int get_ctrl_reg_from_unit_addr(const uint unit_addr) +{ + if (unit_addr < ARRAY_SIZE(ctrl_regs)) + return ctrl_regs[unit_addr]; + return -ENXIO; +} + +static int tps65910_regulator_get_value(struct udevice *dev, + const struct regulator_props *rgp) +{ + int sel, val, vout; + struct tps65910_regulator_pdata *pdata = dev_get_platdata(dev); + int vin = pdata->supply; + + val = pmic_reg_read(dev->parent, rgp->reg); + if (val < 0) + return val; + sel = (val & TPS65910_SEL_MASK) >> 2; + vout = (vin >= *(rgp->vin_min + sel)) ? *(rgp->vout + sel) : 0; + vout = ((val & TPS65910_SUPPLY_STATE_MASK) == 1) ? vout : 0; + + return vout; +} + +static int tps65910_ldo_get_value(struct udevice *dev) +{ + struct tps65910_regulator_pdata *pdata = dev_get_platdata(dev); + int vin; + + if (!pdata) + return 0; + vin = pdata->supply; + + switch (pdata->unit) { + case TPS65910_UNIT_VRTC: + /* VRTC is fixed and can't be turned off */ + return (vin >= 2500000) ? 1830000 : 0; + case TPS65910_UNIT_VDIG1: + return tps65910_regulator_get_value(dev, &ldo_props_vdig1); + case TPS65910_UNIT_VDIG2: + return tps65910_regulator_get_value(dev, &ldo_props_vdig2); + case TPS65910_UNIT_VPLL: + return tps65910_regulator_get_value(dev, &ldo_props_vpll); + case TPS65910_UNIT_VDAC: + return tps65910_regulator_get_value(dev, &ldo_props_vdac); + case TPS65910_UNIT_VAUX1: + return tps65910_regulator_get_value(dev, &ldo_props_vaux1); + case TPS65910_UNIT_VAUX2: + return tps65910_regulator_get_value(dev, &ldo_props_vaux2); + case TPS65910_UNIT_VAUX33: + return tps65910_regulator_get_value(dev, &ldo_props_vaux33); + case TPS65910_UNIT_VMMC: + return tps65910_regulator_get_value(dev, &ldo_props_vmmc); + default: + return 0; + } +} + +static int tps65910_regulator_set_value(struct udevice *dev, + const struct regulator_props *ldo, + int uV) +{ + int val; + int sel = 0; + struct tps65910_regulator_pdata *pdata = dev_get_platdata(dev); + + do { + /* we only allow exact voltage matches */ + if (uV == *(ldo->vout + sel)) + break; + } while (++sel < VOUT_CHOICE_COUNT); + if (sel == VOUT_CHOICE_COUNT) + return -EINVAL; + if (pdata->supply < *(ldo->vin_min + sel)) + return -EINVAL; + + val = pmic_reg_read(dev->parent, ldo->reg); + if (val < 0) + return val; + val &= ~TPS65910_SEL_MASK; + val |= sel << 2; + return pmic_reg_write(dev->parent, ldo->reg, val); +} + +static int tps65910_ldo_set_value(struct udevice *dev, int uV) +{ + struct tps65910_regulator_pdata *pdata = dev_get_platdata(dev); + int vin = pdata->supply; + + switch (pdata->unit) { + case TPS65910_UNIT_VRTC: + /* VRTC is fixed to 1.83V and can't be turned off */ + if (vin < 2500000) + return -EINVAL; + return 0; + case TPS65910_UNIT_VDIG1: + return tps65910_regulator_set_value(dev, &ldo_props_vdig1, uV); + case TPS65910_UNIT_VDIG2: + return tps65910_regulator_set_value(dev, &ldo_props_vdig2, uV); + case TPS65910_UNIT_VPLL: + return tps65910_regulator_set_value(dev, &ldo_props_vpll, uV); + case TPS65910_UNIT_VDAC: + return tps65910_regulator_set_value(dev, &ldo_props_vdac, uV); + case TPS65910_UNIT_VAUX1: + return tps65910_regulator_set_value(dev, &ldo_props_vaux1, uV); + case TPS65910_UNIT_VAUX2: + return tps65910_regulator_set_value(dev, &ldo_props_vaux2, uV); + case TPS65910_UNIT_VAUX33: + return tps65910_regulator_set_value(dev, &ldo_props_vaux33, uV); + case TPS65910_UNIT_VMMC: + return tps65910_regulator_set_value(dev, &ldo_props_vmmc, uV); + default: + return 0; + } +} + +static int tps65910_get_enable(struct udevice *dev) +{ + int reg, val; + struct tps65910_regulator_pdata *pdata = dev_get_platdata(dev); + + reg = get_ctrl_reg_from_unit_addr(pdata->unit); + if (reg < 0) + return reg; + + val = pmic_reg_read(dev->parent, reg); + if (val < 0) + return val; + + /* bits 1:0 of regulator control register define state */ + return ((val & TPS65910_SUPPLY_STATE_MASK) == 1); +} + +static int tps65910_set_enable(struct udevice *dev, bool enable) +{ + int reg; + uint clr, set; + struct tps65910_regulator_pdata *pdata = dev_get_platdata(dev); + + reg = get_ctrl_reg_from_unit_addr(pdata->unit); + if (reg < 0) + return reg; + + if (enable) { + clr = TPS65910_SUPPLY_STATE_MASK & ~TPS65910_SUPPLY_STATE_ON; + set = TPS65910_SUPPLY_STATE_MASK & TPS65910_SUPPLY_STATE_ON; + } else { + clr = TPS65910_SUPPLY_STATE_MASK & ~TPS65910_SUPPLY_STATE_OFF; + set = TPS65910_SUPPLY_STATE_MASK & TPS65910_SUPPLY_STATE_OFF; + } + return pmic_clrsetbits(dev->parent, reg, clr, set); +} + +static int buck_get_vdd1_vdd2_value(struct udevice *dev, int reg_vdd) +{ + int gain; + int val = pmic_reg_read(dev, reg_vdd); + + if (val < 0) + return val; + gain = (val & TPS65910_GAIN_SEL_MASK) >> 6; + gain = (gain == 0) ? 1 : gain; + val = pmic_reg_read(dev, reg_vdd + 1); + if (val < 0) + return val; + if (val & TPS65910_VDD_SR_MASK) + /* use smart reflex value instead */ + val = pmic_reg_read(dev, reg_vdd + 2); + if (val < 0) + return val; + return (562500 + (val & TPS65910_VDD_SEL_MASK) * 12500) * gain; +} + +static int tps65910_buck_get_value(struct udevice *dev) +{ + struct tps65910_regulator_pdata *pdata = dev_get_platdata(dev); + + switch (pdata->unit) { + case TPS65910_UNIT_VIO: + return tps65910_regulator_get_value(dev, &smps_props_vio); + case TPS65910_UNIT_VDD1: + return buck_get_vdd1_vdd2_value(dev->parent, TPS65910_REG_VDD1); + case TPS65910_UNIT_VDD2: + return buck_get_vdd1_vdd2_value(dev->parent, TPS65910_REG_VDD2); + default: + return 0; + } +} + +static int buck_set_vdd1_vdd2_value(struct udevice *dev, int uV) +{ + int ret, reg_vdd, gain; + int val; + struct dm_regulator_uclass_platdata *uc_pdata; + struct tps65910_regulator_pdata *pdata = dev_get_platdata(dev); + + switch (pdata->unit) { + case TPS65910_UNIT_VDD1: + reg_vdd = TPS65910_REG_VDD1; + break; + case TPS65910_UNIT_VDD2: + reg_vdd = TPS65910_REG_VDD2; + break; + default: + return -EINVAL; + } + uc_pdata = dev_get_uclass_platdata(dev); + + /* check setpoint is within limits */ + if (uV < uc_pdata->min_uV) { + error("voltage %duV for %s too low\n", uV, dev->name); + return -EINVAL; + } + if (uV > uc_pdata->max_uV) { + error("voltage %duV for %s too high\n", uV, dev->name); + return -EINVAL; + } + + val = pmic_reg_read(dev->parent, reg_vdd); + if (val < 0) + return val; + gain = (val & TPS65910_GAIN_SEL_MASK) >> 6; + gain = (gain == 0) ? 1 : gain; + val = ((uV / gain) - 562500) / 12500; + if (val < TPS65910_VDD_SEL_MIN || val > TPS65910_VDD_SEL_MAX) + /* + * Neither do we change the gain, nor do we allow shutdown or + * any approximate value (for now) + */ + return -EPERM; + val &= TPS65910_VDD_SEL_MASK; + ret = pmic_reg_write(dev->parent, reg_vdd + 1, val); + if (ret) + return ret; + return 0; +} + +static int tps65910_buck_set_value(struct udevice *dev, int uV) +{ + struct tps65910_regulator_pdata *pdata = dev_get_platdata(dev); + + if (pdata->unit == TPS65910_UNIT_VIO) + return tps65910_regulator_set_value(dev, &smps_props_vio, uV); + + return buck_set_vdd1_vdd2_value(dev, uV); +} + +static int tps65910_boost_get_value(struct udevice *dev) +{ + int vout; + struct tps65910_regulator_pdata *pdata = dev_get_platdata(dev); + + vout = (pdata->supply >= 3000000) ? 5000000 : 0; + return vout; +} + +static int tps65910_regulator_ofdata_to_platdata(struct udevice *dev) +{ + struct udevice *supply; + int ret; + const char *supply_name; + struct tps65910_regulator_pdata *pdata = dev_get_platdata(dev); + + pdata->unit = dev_get_driver_data(dev); + if (pdata->unit > TPS65910_UNIT_VMMC) + return -EINVAL; + supply_name = supply_names[regulator_supplies[pdata->unit]]; + + debug("Looking up supply power %s\n", supply_name); + ret = device_get_supply_regulator(dev->parent, supply_name, &supply); + if (ret) { + debug(" missing supply power %s\n", supply_name); + return ret; + } + pdata->supply = regulator_get_value(supply); + if (pdata->supply < 0) { + debug(" invalid supply voltage for regulator %s\n", + supply->name); + return -EINVAL; + } + + return 0; +} + +static const struct dm_regulator_ops tps65910_boost_ops = { + .get_value = tps65910_boost_get_value, + .get_enable = tps65910_get_enable, + .set_enable = tps65910_set_enable, +}; + +U_BOOT_DRIVER(tps65910_boost) = { + .name = TPS65910_BOOST_DRIVER, + .id = UCLASS_REGULATOR, + .ops = &tps65910_boost_ops, + .platdata_auto_alloc_size = sizeof(struct tps65910_regulator_pdata), + .ofdata_to_platdata = tps65910_regulator_ofdata_to_platdata, +}; + +static const struct dm_regulator_ops tps65910_buck_ops = { + .get_value = tps65910_buck_get_value, + .set_value = tps65910_buck_set_value, + .get_enable = tps65910_get_enable, + .set_enable = tps65910_set_enable, +}; + +U_BOOT_DRIVER(tps65910_buck) = { + .name = TPS65910_BUCK_DRIVER, + .id = UCLASS_REGULATOR, + .ops = &tps65910_buck_ops, + .platdata_auto_alloc_size = sizeof(struct tps65910_regulator_pdata), + .ofdata_to_platdata = tps65910_regulator_ofdata_to_platdata, +}; + +static const struct dm_regulator_ops tps65910_ldo_ops = { + .get_value = tps65910_ldo_get_value, + .set_value = tps65910_ldo_set_value, + .get_enable = tps65910_get_enable, + .set_enable = tps65910_set_enable, +}; + +U_BOOT_DRIVER(tps65910_ldo) = { + .name = TPS65910_LDO_DRIVER, + .id = UCLASS_REGULATOR, + .ops = &tps65910_ldo_ops, + .platdata_auto_alloc_size = sizeof(struct tps65910_regulator_pdata), + .ofdata_to_platdata = tps65910_regulator_ofdata_to_platdata, +}; diff --git a/include/power/tps65910_pmic.h b/include/power/tps65910_pmic.h new file mode 100644 index 000000000000..e8d9ffaa9fb9 --- /dev/null +++ b/include/power/tps65910_pmic.h @@ -0,0 +1,130 @@ +/* + * Copyright (C) EETS GmbH, 2017, Felix Brack + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef __TPS65910_PMIC_H_ +#define __TPS65910_PMIC_H_ + +#define TPS65910_I2C_SEL_MASK (0x1 << 4) +#define TPS65910_VDD_SR_MASK (0x1 << 7) +#define TPS65910_GAIN_SEL_MASK (0x3 << 6) +#define TPS65910_VDD_SEL_MASK 0x7f +#define TPS65910_VDD_SEL_MIN 3 +#define TPS65910_VDD_SEL_MAX 75 +#define TPS65910_SEL_MASK (0x3 << 2) +#define TPS65910_SUPPLY_STATE_MASK 0x3 +#define TPS65910_SUPPLY_STATE_OFF 0x0 +#define TPS65910_SUPPLY_STATE_ON 0x1 + +/* i2c registers */ +enum { + TPS65910_REG_RTC_SEC = 0x00, + TPS65910_REG_RTC_MIN, + TPS65910_REG_RTC_HOUR, + TPS65910_REG_RTC_DAY, + TPS65910_REG_RTC_MONTH, + TPS65910_REG_RTC_YEAR, + TPS65910_REG_RTC_WEEK, + TPS65910_REG_RTC_ALARM_SEC = 0x08, + TPS65910_REG_RTC_ALARM_MIN, + TPS65910_REG_RTC_ALARM_HOUR, + TPS65910_REG_RTC_ALARM_DAY, + TPS65910_REG_RTC_ALARM_MONTH, + TPS65910_REG_RTC_ALARM_YEAR, + TPS65910_REG_RTC_CTRL = 0x10, + TPS65910_REG_RTC_STAT, + TPS65910_REG_RTC_INT, + TPS65910_REG_RTC_COMP_LSB, + TPS65910_REG_RTC_COMP_MSB, + TPS65910_REG_RTC_RESISTOR_PRG, + TPS65910_REG_RTC_RESET_STAT, + TPS65910_REG_BACKUP1, + TPS65910_REG_BACKUP2, + TPS65910_REG_BACKUP3, + TPS65910_REG_BACKUP4, + TPS65910_REG_BACKUP5, + TPS65910_REG_PUADEN, + TPS65910_REG_REF, + TPS65910_REG_VRTC, + TPS65910_REG_VIO = 0x20, + TPS65910_REG_VDD1, + TPS65910_REG_VDD1_VAL, + TPS65910_REG_VDD1_VAL_SR, + TPS65910_REG_VDD2, + TPS65910_REG_VDD2_VAL, + TPS65910_REG_VDD2_VAL_SR, + TPS65910_REG_VDD3, + TPS65910_REG_VDIG1 = 0x30, + TPS65910_REG_VDIG2, + TPS65910_REG_VAUX1, + TPS65910_REG_VAUX2, + TPS65910_REG_VAUX33, + TPS65910_REG_VMMC, + TPS65910_REG_VPLL, + TPS65910_REG_VDAC, + TPS65910_REG_THERM, + TPS65910_REG_BATTERY_BACKUP_CHARGE, + TPS65910_REG_DCDC_CTRL = 0x3e, + TPS65910_REG_DEVICE_CTRL, + TPS65910_REG_DEVICE_CTRL2, + TPS65910_REG_SLEEP_KEEP_LDO_ON, + TPS65910_REG_SLEEP_KEEP_RES_ON, + TPS65910_REG_SLEEP_SET_LDO_OFF, + TPS65910_REG_SLEEP_SET_RES_OFF, + TPS65910_REG_EN1_LDO_ASS, + TPS65910_REG_EM1_SMPS_ASS, + TPS65910_REG_EN2_LDO_ASS, + TPS65910_REG_EM2_SMPS_ASS, + TPS65910_REG_INT_STAT = 0x50, + TPS65910_REG_INT_MASK, + TPS65910_REG_INT_STAT2, + TPS65910_REG_INT_MASK2, + TPS65910_REG_GPIO = 0x60, + TPS65910_REG_JTAGREVNUM = 0x80, + TPS65910_NUM_REGS +}; + +/* chip supplies */ +enum { + TPS65910_SUPPLY_VCCIO = 0x00, + TPS65910_SUPPLY_VCC1, + TPS65910_SUPPLY_VCC2, + TPS65910_SUPPLY_VCC3, + TPS65910_SUPPLY_VCC4, + TPS65910_SUPPLY_VCC5, + TPS65910_SUPPLY_VCC6, + TPS65910_SUPPLY_VCC7, + TPS65910_NUM_SUPPLIES +}; + +/* regulator unit numbers */ +enum { + TPS65910_UNIT_VRTC = 0x00, + TPS65910_UNIT_VIO, + TPS65910_UNIT_VDD1, + TPS65910_UNIT_VDD2, + TPS65910_UNIT_VDD3, + TPS65910_UNIT_VDIG1, + TPS65910_UNIT_VDIG2, + TPS65910_UNIT_VPLL, + TPS65910_UNIT_VDAC, + TPS65910_UNIT_VAUX1, + TPS65910_UNIT_VAUX2, + TPS65910_UNIT_VAUX33, + TPS65910_UNIT_VMMC, +}; + +/* platform data */ +struct tps65910_regulator_pdata { + u32 supply; /* regulator supply voltage in uV */ + uint unit; /* unit-address according to DT */ +}; + +/* driver names */ +#define TPS65910_BUCK_DRIVER "tps65910_buck" +#define TPS65910_BOOST_DRIVER "tps65910_boost" +#define TPS65910_LDO_DRIVER "tps65910_ldo" + +#endif /* __TPS65910_PMIC_H_ */