Re: F2-NAS2 and NSA325 contributions
Hi Henry, On Thu, Jun 29, 2023 at 7:52 PM Henry Smith wrote: > > Oh hello! > > Nice to hear from the person I wanted to contact. I based my work on > your fork of the marvell repo. > My priority is to get the new device into the tree so I don't lose it > and other people can benefit. Please proceed at your own pace. > Whatever you contributed to 2023 can be considered at a later time. > > I have a branch that compiles and boots on F2-NAS2 from TerraMaster. > Powering the thing off, wakeup on a schedule and RTC is implemented > through an extra uC, on/off button is not connected to the SoC I > think. On the NSA325, the Power button is connected to gpio1 14 (mpp 46). It sounds like the F2-NAS2 is using the UART1 port for that purpose instead. I came across a few boards like that. > This uC is most likely connected to SoC via UART1 and receives > commands from a small daemon controlled by a tiny client program. I > wanted to make sure this is not something that would prevent you from > accepting a new board. > It does turn on. Pressing the power button when the device is on shuts > the power off after 5-10s. Time is never set either so I used ntp. Now Don't know about the F2-NAS2 , but the NSA325 has RTC (PCF8563 chip), and it is already supported by current u-boot. > I see u-boot supports that as well but unsure if that would somehow > get passed to the kernel with no real RTC. For these things to work, > the serial commands would have to get deciphered and implemented in > u-boot itself or the OS like it is in TOS from the vendor. I can try > to do it or disassemble at a later time. > If there is no RTC on the F2-NAS2, you can use CONFIG_RTC_EMULATION and CONFIG_CMD_SNTP. But rootfs will need fake-hwclock anyway.. All the best, Tony > czw., 29 cze 2023 o 21:01 Tony Dinh napisał(a): > > > > Hi Henry, > > > > On Thu, Jun 29, 2023 at 6:53 PM Tony Dinh wrote: > > > > > > Hi Heny, > > > > > > On Thu, Jun 29, 2023 at 6:01 PM Henry Smith wrote: > > > > > > > > Hello, > > > > > > > > I'm a hobbyist that has been playing with Linkstation LS-WVL years > > > > ago. After frying UART on that, I got Terramaster F2-NAS2 later and > > > > got u-boot from the main tree to work on it. > > > > > > > > Both landed in the closet until I dug up F2-NAS2 recently. My original > > > > source code modifications and images were nowhere to be found so I > > > > started from scratch. > > > > > > > > First tried to compile and run every defconfig available that uses > > > > Kirkwood SoC: > > > > u-boot # for config in $(grep -i kirkwood ./configs -r | cut -d ":" > > > > -f1 | cut -d "/" -f3 | sort -u); do make clean; make distclean; > > > > ARCH=arm CROSS_COMPILE=arm-none-eabi- make $config && ARCH=arm > > > > CROSS_COMPILE=arm-none-eabi- make -j64; echo $config; read; done > > > > > > > > Unfortunately, that didn't work. Then I saw the following: > > > > "...By the way this box is a close cousin of the NSA325 (Kirkwood)..." > > > > Source: > > > > https://forum.doozan.com/read.php?3,49804,49964#msg-49964 > > > > > > > > With that in mind, I've found the first compiled version I could lay > > > > my hands on: > > > > https://forum.doozan.com/read.php?3,12381 > > > > Actual file: > > > > https://bitly.com/2xnfGmv > > > > > > > > The file worked, here is the source: > > > > https://github.com/mibodhi/u-boot-kirkwood > > > > > > > > > > That is my Kirkwood u-boot GitHub for 2017.07. So it would not compile > > > with current u-boot. > > > > > > > The thing didn't compile and threw errors about libfdt: > > > > https://github.com/hardkernel/u-boot/issues/73 > > > > > > > > This is when I started porting it to the main u-boot tree from that > > > > fork. I got it to boot on Terramaster F2-NAS2. > > > > > > > > To continue, I have 2 questions: > > > > 1) Original changes ported to the original tree were for NSA325. Can > > > > somebody test them on their own box or that's not something you would > > > > do? > > > > > > I've planned to add support to mainline u-boot for ZyXEL NSA325 board. > > > But I've been busy for a while so have not got time to do it. I've > > > already released the u-boot 2023.04. binary in doozan forum. > > > > > > https://forum.doozan.com/read.php?3,12381 > > > "2023.04 U-Boot Kirkwood - ZyXEL NSA325. This version adds the > > > capability of booting with the rootfs attached to the USB 3.0 port in > > > the front, among other capabilities" > > > > > > > 2) I have changes for Terramaster F2-NAS2 derived from #1 as well. How > > > > do I submit them since the PR pointed me to the mailing list: > > > > https://github.com/u-boot/u-boot/pull/326 > > > > > > Perhaps, you could wait until I send the NSA325 patch to this ML. And > > > then use that as a template to create the patch for F2-NAS2? We only > > > submit patches using the mailing list. And the patch will be reviewed > > > by Stefan and/or others. Of course it's up to you, but I think it > > > might be best that we could collaborate. > > > > > > All the
Re: [PATCH 10/12] binman: btool: Add Xilinx Bootgen btool
Hi,
On Thu, 29 Jun 2023 at 15:59, wrote:
>
> From: Lukas Funke
>
> Add the Xilinx Bootgen as bintool. Xilinx Bootgen is used to create
> bootable SPL (FSBL in Xilinx terms) images for Zynq/ZynqMP devices. The
> btool creates a signed version of the SPL.
>
> Signed-off-by: Lukas Funke
> ---
>
> tools/binman/btool/bootgen.py | 82 +++
> 1 file changed, 82 insertions(+)
> create mode 100644 tools/binman/btool/bootgen.py
>
> diff --git a/tools/binman/btool/bootgen.py b/tools/binman/btool/bootgen.py
> new file mode 100644
> index 00..8bc727a54f
> --- /dev/null
> +++ b/tools/binman/btool/bootgen.py
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (C) 2023 Weidmüller Interface GmbH & Co. KG
> +# Lukas Funke
> +#
> +"""Bintool implementation for bootgen
> +
> +bootgen allows creating bootable SPL for Zynq(MP)
> +
> +Documentation is available via::
> +https://www.xilinx.com/support/documents/sw_manuals/xilinx2022_1/ug1283-bootgen-user-guide.pdf
But you need to create some info here. It is good to have that link,
but we also need some about of info about it here.
Overall this whole patch needs more docs and detail.
> +
> +Source code is available at:
> +
> +https://github.com/Xilinx/bootgen
> +
> +"""
> +import tempfile
> +
> +from binman import bintool
> +from u_boot_pylib import tools
> +
> +# pylint: disable=C0103
> +class Bintoolbootgen(bintool.Bintool):
> +"""Generate bootable fsbl image for zynq/zynqmp
> +
> +This bintools supports running Xilinx "bootgen" in order
> +to generate a bootable, authenticated image form an SPL.
> +
> +"""
> +def __init__(self, name):
> +super().__init__(name, 'Xilinx Bootgen',
> + version_regex=r'^\*\*\*\*\*\* Xilinx Bootgen',
> + version_args='')
> +
> +# pylint: disable=R0913
> +def sign(self, arch, spl_elf_fname, pmufw_elf_fname,
> + psk_fname, ssk_fname, fsbl_config, auth_params, output_fname):
> +""" Sign FSBL(SPL) elf file and bundle it with pmu firmware
> +to a bootable image
> +
> +Args:
> +arch (str): Xilinx SoC architecture
what options are valid?
> +spl_elf_fname (str): Filename of FSBL ELF file
what is FSBL?
> +pmufw_elf_fname (str): Filename pmu firmware
what is pmu?
> +psk_fname (str): Filename of the primary secret key
> +ssk_fname (str): Filename of the secondary secret key
why are there two keys? What format is used for these keys?
> +fsbl_config (str): FSBL config options
what options are available? What are valid vaulues for this arg?
> +auth_params (str): Authentication parameter
what are valid values?
> +output_fname (str): Filename where bootgen should write the
> result
This should really be handled automatically, I think. See how the
mkimage etype creates an output filename.
> +"""
> +
> +_fsbl_config = f"[fsbl_config] {fsbl_config}" if fsbl_config else ""
> +_auth_params = f"[auth_params] {auth_params}" if auth_params else ""
> +
> +bif_template = f"""u_boot_spl_aes_rsa: {{
> +[pskfile] {psk_fname}
> +[sskfile] {ssk_fname}
> +{_fsbl_config}
> +{_auth_params}
> +[ bootloader,
> + authentication = rsa,
> + destination_cpu=a53-0] {spl_elf_fname}
> +[pmufw_image] {pmufw_elf_fname}
> +}}"""
> +args = ["-arch", arch]
> +
> +with tempfile.NamedTemporaryFile(suffix=".bif",
> + dir=tools.get_output_dir()) as bif:
Please use a deterministic name - see mkimage etype for an example.
> +tools.write_file(bif.name, bif_template, binary=False)
> +args += ["-image", bif.name, '-w', '-o', output_fname]
> +self.run_cmd(*args)
> +
> +def fetch(self, method):
> +"""Fetch bootgen from git"""
> +if method != bintool.FETCH_BUILD:
> +return None
> +
> +result = self.build_from_git(
> +'https://github.com/Xilinx/bootgen',
> +'all',
> +'build/bootgen/bootgen')
> +return result
> --
> 2.30.2
>
Regards,
Simon
Re: [PATCH 09/12] binman: doc: Add documentation for Xilinx Bootgen bintool
On Thu, 29 Jun 2023 at 15:59, wrote: > > From: Lukas Funke > > Add documentation for the 'bootgen' bintool > > Signed-off-by: Lukas Funke > --- > > tools/binman/bintools.rst | 12 > 1 file changed, 12 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH 08/12] binman: etype: Add u_boot_spl_pubkey_dtb etype
On Thu, 29 Jun 2023 at 15:59, wrote:
>
> From: Lukas Funke
>
> This adds a new etype 'u_boot_spl_pubkey_dtb'. The etype adds the public
> key from a certificate to the dtb. This creates a '/signature' node which
> is turn contains the fields which make up the public key. Usually this
> is done by 'mkimage -K'. However, 'binman sign' does not add the public
> key to the SPL. This is why the pubkey is added using this etype.
>
> The etype calls the underlying 'fdt_add_pubkey' tool.
>
> Signed-off-by: Lukas Funke
> ---
>
> tools/binman/etype/u_boot_spl_pubkey_dtb.py | 105
> 1 file changed, 105 insertions(+)
> create mode 100644 tools/binman/etype/u_boot_spl_pubkey_dtb.py
Please can you use 'binman entry-docs >tools/binman/entries.rst' and
add to patch?
>
> diff --git a/tools/binman/etype/u_boot_spl_pubkey_dtb.py
> b/tools/binman/etype/u_boot_spl_pubkey_dtb.py
> new file mode 100644
> index 00..25aa817975
> --- /dev/null
> +++ b/tools/binman/etype/u_boot_spl_pubkey_dtb.py
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2023 Weidmueller GmbH
> +# Written by Lukas Funke
> +#
> +# Entry-type module for 'u-boot-spl-pubkey.dtb'
> +#
> +
> +import tempfile
> +import os
> +
> +from binman.etype.blob_dtb import Entry_blob_dtb
> +
> +from dtoc import fdt_util
> +
> +from u_boot_pylib import tools
> +
> +# pylint: disable=C0103
> +class Entry_u_boot_spl_pubkey_dtb(Entry_blob_dtb):
> +"""U-Boot SPL device tree including public key
> +
> +Properties / Entry arguments:
> +- key-name: Public key name without extension (e.g. .crt). Default is
> +determined by underlying bintool (fdt_add_pubkey),
> +usually 'key'
> +- algo: (Optional) Algorithm used for signing. Default is determined
> by
> +underlying bintool (fdt_add_pubkey), usually 'sha1,rsa2048'
> +- required: (Optional) If present this indicates that the key must be
> +verified for the image / configuration to be
> +considered valid
> +
> +The following example shows an image containing an SPL which
> +is packed together with the dtb. Binman will add a signature
> +node to the dtb:
> +
> +image {
> +...
> +spl {
> +filename = "spl.bin"
> +
> +u_boot_spl_nodtb {
> +};
> +u_boot_spl_pubkey_dtb {
> +algo = "sha384,rsa4096";
> +required = "conf";
> +key-name = "dev";
> +};
> +};
> +...
> +}
> +"""
> +
> +def __init__(self, section, etype, node):
> +# Put this here to allow entry-docs and help to work without libfdt
> +global state
> +from binman import state
> +
> +super().__init__(section, etype, node)
> +self.required_props = ['key-name']
> +self.fdt_add_pubkey = None
> +self._algo = fdt_util.GetString(self._node, 'algo')
> +self._required = fdt_util.GetString(self._node, 'required')
> +self._keyname = fdt_util.GetString(self._node, 'key-name')
> +
> +def ObtainContents(self, fake_size=0):
> +""" Add public key which is pointed out by
Please check comment style. The first line should a summary, then a
blank line, then more info
> +'key-name' to node 'signature' in the spl-dtb
> +
> +This is equivalent to the '-K' option of 'mkimage'
> +
> +Args:
> +fake_size (int): unused
> +"""
> +
> +# We don't pass fake_size and skip_entry upwards
> +# because this is currently not support by the blob type
supported
> +super().ObtainContents()
> +
> +with tempfile.NamedTemporaryFile(prefix=os.path.basename(
> + self.GetFdtEtype()),
> + dir=tools.get_output_dir())\
> + as pubkey_tdb:
> +tools.write_file(pubkey_tdb.name, self.GetData())
> +keyname = tools.get_input_filename(self._keyname + ".crt")
> +self.fdt_add_pubkey.run(pubkey_tdb.name,
> +os.path.dirname(keyname),
> +self._keyname,
> +self._required, self._algo)
> +dtb = tools.read_file(pubkey_tdb.name)
> +self.SetContents(dtb)
> +state.UpdateFdtContents(self.GetFdtEtype(), dtb)
> +
> +return True
> +
> +# pylint: disable=R0201,C0116
> +def GetDefaultFilename(self):
> +return 'spl/u-boot-spl-pubkey.dtb'
> +
> +# pylint: disable=R0201,C0116
> +def GetFdtEtype(self):
> +return 'u-boot-spl-dtb'
> +
> +# pylint: disable=R0201,C0116
> +def AddBintools(self, btools):
> +super().AddBintools(btools)
> +
Re: [PATCH 07/12] binman: btool: Add fdt_add_pubkey as btool
On Thu, 29 Jun 2023 at 15:59, wrote: > > From: Lukas Funke > > Add btool which calls 'fdt_add_pubkey' > > Signed-off-by: Lukas Funke > --- > > tools/binman/btool/fdt_add_pubkey.py | 67 > 1 file changed, 67 insertions(+) > create mode 100644 tools/binman/btool/fdt_add_pubkey.py Reviewed-by: Simon Glass
Re: [PATCH 06/12] binman: ftest: Add test for u_boot_spl_pubkey_dtb
On Thu, 29 Jun 2023 at 15:59, wrote:
>
> From: Lukas Funke
>
> Add test for u_boot_spl_pubkey_dtb. The test adds a public key to the
> dtb and checks if the required nodes will be added to the images dtb.
>
> Signed-off-by: Lukas Funke
> ---
>
> tools/binman/ftest.py| 32
> tools/binman/test/281_spl_pubkey_dtb.dts | 16
> 2 files changed, 48 insertions(+)
> create mode 100644 tools/binman/test/281_spl_pubkey_dtb.dts
Reviewed-by: Simon Glass
nit below
>
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 43b4f850a6..3bd09d3fea 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -638,6 +638,16 @@ class TestFunctional(unittest.TestCase):
> TestFunctional._MakeInputFile('vpl/u-boot-vpl',
> tools.read_file(cls.ElfTestFile(src_fname)))
>
> +@classmethod
> +def _SetupPmuFwlElf(cls, src_fname='bss_data'):
> +"""Set up an ELF file with a '_dt_ucode_base_size' symbol
> +
> +Args:
> +Filename of ELF file to use as VPL
> +"""
> +TestFunctional._MakeInputFile('pmu-firmware.elf',
> +tools.read_file(cls.ElfTestFile(src_fname)))
> +
> @classmethod
> def _SetupDescriptor(cls):
> with open(cls.TestFile('descriptor.bin'), 'rb') as fd:
> @@ -6677,5 +6687,27 @@ fdt fdtmapExtract the
> devicetree blob from the fdtmap
> self.assertIn("Node '/fit': Missing tool: 'mkimage'",
> str(e.exception))
>
>
> +def testSplPubkeyDtb(self):
> + """Test u_boot_spl_pubkey_dtb etype"""
> + data = tools.read_file(self.TestFile("key.pem"))
> + self._MakeInputFile("key.crt", data)
> + self._DoReadFileRealDtb('281_spl_pubkey_dtb.dts')
> + image = control.images['image']
> + entries = image.GetEntries()
> + dtb_entry = entries['u_boot_spl_pubkey_dtb']
> + dtb_data = dtb_entry.GetData()
> + dtb = fdt.Fdt.FromData(dtb_data)
> + dtb.Scan()
> +
> + signature_node = dtb.GetNode('/signature')
> + self.assertIsNotNone(signature_node)
> + key_node = signature_node.FindNode("key-key")
> + self.assertIsNotNone(key_node)
> + self.assertEqual(fdt_util.GetString(key_node, "required"),
> + "conf")
> + self.assertEqual(fdt_util.GetString(key_node, "algo"),
> + "sha384,rsa4096")
> + self.assertEqual(fdt_util.GetString(key_node, "key-name-hint"),
> + "key")
> if __name__ == "__main__":
> unittest.main()
> diff --git a/tools/binman/test/281_spl_pubkey_dtb.dts
> b/tools/binman/test/281_spl_pubkey_dtb.dts
> new file mode 100644
> index 00..5a2952ed7d
> --- /dev/null
> +++ b/tools/binman/test/281_spl_pubkey_dtb.dts
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + binman {
> + u_boot_spl_pubkey_dtb {
please use - instead of _
> + algo = "sha384,rsa4096";
> + required = "conf";
> + key-name = "key";
> + };
> + };
> +};
> --
> 2.30.2
>
Re: F2-NAS2 and NSA325 contributions
Hi Henry, On Thu, Jun 29, 2023 at 6:53 PM Tony Dinh wrote: > > Hi Heny, > > On Thu, Jun 29, 2023 at 6:01 PM Henry Smith wrote: > > > > Hello, > > > > I'm a hobbyist that has been playing with Linkstation LS-WVL years > > ago. After frying UART on that, I got Terramaster F2-NAS2 later and > > got u-boot from the main tree to work on it. > > > > Both landed in the closet until I dug up F2-NAS2 recently. My original > > source code modifications and images were nowhere to be found so I > > started from scratch. > > > > First tried to compile and run every defconfig available that uses Kirkwood > > SoC: > > u-boot # for config in $(grep -i kirkwood ./configs -r | cut -d ":" > > -f1 | cut -d "/" -f3 | sort -u); do make clean; make distclean; > > ARCH=arm CROSS_COMPILE=arm-none-eabi- make $config && ARCH=arm > > CROSS_COMPILE=arm-none-eabi- make -j64; echo $config; read; done > > > > Unfortunately, that didn't work. Then I saw the following: > > "...By the way this box is a close cousin of the NSA325 (Kirkwood)..." > > Source: > > https://forum.doozan.com/read.php?3,49804,49964#msg-49964 > > > > With that in mind, I've found the first compiled version I could lay > > my hands on: > > https://forum.doozan.com/read.php?3,12381 > > Actual file: > > https://bitly.com/2xnfGmv > > > > The file worked, here is the source: > > https://github.com/mibodhi/u-boot-kirkwood > > > > That is my Kirkwood u-boot GitHub for 2017.07. So it would not compile > with current u-boot. > > > The thing didn't compile and threw errors about libfdt: > > https://github.com/hardkernel/u-boot/issues/73 > > > > This is when I started porting it to the main u-boot tree from that > > fork. I got it to boot on Terramaster F2-NAS2. > > > > To continue, I have 2 questions: > > 1) Original changes ported to the original tree were for NSA325. Can > > somebody test them on their own box or that's not something you would > > do? > > I've planned to add support to mainline u-boot for ZyXEL NSA325 board. > But I've been busy for a while so have not got time to do it. I've > already released the u-boot 2023.04. binary in doozan forum. > > https://forum.doozan.com/read.php?3,12381 > "2023.04 U-Boot Kirkwood - ZyXEL NSA325. This version adds the > capability of booting with the rootfs attached to the USB 3.0 port in > the front, among other capabilities" > > > 2) I have changes for Terramaster F2-NAS2 derived from #1 as well. How > > do I submit them since the PR pointed me to the mailing list: > > https://github.com/u-boot/u-boot/pull/326 > > Perhaps, you could wait until I send the NSA325 patch to this ML. And > then use that as a template to create the patch for F2-NAS2? We only > submit patches using the mailing list. And the patch will be reviewed > by Stefan and/or others. Of course it's up to you, but I think it > might be best that we could collaborate. > > All the best, > Tony > > > > > Please do not look at the changes in the PR, they are old and > > irrelevant. I will close or update it, depending on what I hear back. > > > > Best, > > > > - Henry Sorry I've misspelled your name previously :) And also just to let you know that I'll be quite occupied with work in the next several weeks, so I might not be able to follow up right away. All the best, Tony
Re: F2-NAS2 and NSA325 contributions
Hi Heny, On Thu, Jun 29, 2023 at 6:01 PM Henry Smith wrote: > > Hello, > > I'm a hobbyist that has been playing with Linkstation LS-WVL years > ago. After frying UART on that, I got Terramaster F2-NAS2 later and > got u-boot from the main tree to work on it. > > Both landed in the closet until I dug up F2-NAS2 recently. My original > source code modifications and images were nowhere to be found so I > started from scratch. > > First tried to compile and run every defconfig available that uses Kirkwood > SoC: > u-boot # for config in $(grep -i kirkwood ./configs -r | cut -d ":" > -f1 | cut -d "/" -f3 | sort -u); do make clean; make distclean; > ARCH=arm CROSS_COMPILE=arm-none-eabi- make $config && ARCH=arm > CROSS_COMPILE=arm-none-eabi- make -j64; echo $config; read; done > > Unfortunately, that didn't work. Then I saw the following: > "...By the way this box is a close cousin of the NSA325 (Kirkwood)..." > Source: > https://forum.doozan.com/read.php?3,49804,49964#msg-49964 > > With that in mind, I've found the first compiled version I could lay > my hands on: > https://forum.doozan.com/read.php?3,12381 > Actual file: > https://bitly.com/2xnfGmv > > The file worked, here is the source: > https://github.com/mibodhi/u-boot-kirkwood > That is my Kirkwood u-boot GitHub for 2017.07. So it would not compile with current u-boot. > The thing didn't compile and threw errors about libfdt: > https://github.com/hardkernel/u-boot/issues/73 > > This is when I started porting it to the main u-boot tree from that > fork. I got it to boot on Terramaster F2-NAS2. > > To continue, I have 2 questions: > 1) Original changes ported to the original tree were for NSA325. Can > somebody test them on their own box or that's not something you would > do? I've planned to add support to mainline u-boot for ZyXEL NSA325 board. But I've been busy for a while so have not got time to do it. I've already released the u-boot 2023.04. binary in doozan forum. https://forum.doozan.com/read.php?3,12381 "2023.04 U-Boot Kirkwood - ZyXEL NSA325. This version adds the capability of booting with the rootfs attached to the USB 3.0 port in the front, among other capabilities" > 2) I have changes for Terramaster F2-NAS2 derived from #1 as well. How > do I submit them since the PR pointed me to the mailing list: > https://github.com/u-boot/u-boot/pull/326 Perhaps, you could wait until I send the NSA325 patch to this ML. And then use that as a template to create the patch for F2-NAS2? We only submit patches using the mailing list. And the patch will be reviewed by Stefan and/or others. Of course it's up to you, but I think it might be best that we could collaborate. All the best, Tony > > Please do not look at the changes in the PR, they are old and > irrelevant. I will close or update it, depending on what I hear back. > > Best, > > - Henry
Re:Re: [PATCH] rockchip: Restore support for boot scripts in legacy image format
Hi:
在 2023-06-29 22:07:05，"Jonas Karlman" 写道：
>Hi Kever,
>
>On 2023-06-29 12:51, Kever Yang wrote:
>> Hi Jonas,
>>
>> What is the legacy image format, does these boards really use than?
>>
>> I think most of the board using linux distro image.
>
>The legacy image format it the normal boot script format,
>in the documentation it is called Legacy U-Boot image, see [1].
>
>i.e. boot scripts compiled with:
>
> mkimage -T script -n 'Test script' -d boot.txt boot.scr
>
>Such boot scripts are used by some linux distro, e.g. Armbian.
>
>[1]
>https://u-boot.readthedocs.io/en/latest/usage/cmd/source.html#legacy-u-boot-image
>
Yes, this is a common use case, I also run into this case a few weeks ago on
armbian[2]:
The code handle this logic in u-boot is image_locate_script.
[2
]https://github.com/armbian/build/blob/main/config/bootscripts/boot-rockchip.cmd
>Regards,
>Jonas
>
>>
>>
>> Thanks,
>>
>> - Kever
>>
>> On 2023/6/27 03:43, Jonas Karlman wrote:
>>> Use of CONFIG_SPL_FIT_SIGNATURE=y cause CONFIG_LEGACY_IMAGE_FORMAT=n as
>>> default, this prevent boot scripts in legacy image format from working
>>> and was an unintended change in the listed fixes commits:
>>>
>>>Wrong image format for "source" command
>>>
>>> Add CONFIG_LEGACY_IMAGE_FORMAT=y to defconfig for affected boards to
>>> restore support for boot scripts in legacy image format.
>>>
>>> Fixes: 3bf8e4080763 ("board: rockchip: add Radxa ROCK5B Rk3588 board")
>>> Fixes: cf777572ca31 ("rockchip: rockpro64: Use SDMA to boost eMMC
>>> performance")
>>> Fixes: 6e2b8344d60c ("rockchip: rock-pi-4: Use SDMA to boost eMMC
>>> performance")
>>> Fixes: 1bf49d5a4a7c ("rockchip: rk3566-radxa-cm3-io: Update defconfig")
>>> Fixes: 703c170b40f2 ("rockchip: rk3568-evb: Update defconfig")
>>> Fixes: 68000f750acd ("rockchip: rk3568-rock-3a: Update defconfig")
>>> Fixes: 6fb02589a608 ("rockchip: rk3588-evb: Update defconfig")
>>> Signed-off-by: Jonas Karlman
>>> ---
>>> configs/evb-rk3568_defconfig | 1 +
>>> configs/evb-rk3588_defconfig | 1 +
>>> configs/radxa-cm3-io-rk3566_defconfig | 1 +
>>> configs/rock-3a-rk3568_defconfig | 1 +
>>> configs/rock-pi-4-rk3399_defconfig| 1 +
>>> configs/rock5b-rk3588_defconfig | 1 +
>>> configs/rockpro64-rk3399_defconfig| 1 +
>>> 7 files changed, 7 insertions(+)
>>>
>>> diff --git a/configs/evb-rk3568_defconfig b/configs/evb-rk3568_defconfig
>>> index 07819d105441..5f3fab7304c2 100644
>>> --- a/configs/evb-rk3568_defconfig
>>> +++ b/configs/evb-rk3568_defconfig
>>> @@ -22,6 +22,7 @@ CONFIG_FIT=y
>>> CONFIG_FIT_VERBOSE=y
>>> CONFIG_SPL_FIT_SIGNATURE=y
>>> CONFIG_SPL_LOAD_FIT=y
>>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>>> CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-evb.dtb"
>>> # CONFIG_DISPLAY_CPUINFO is not set
>>> CONFIG_DISPLAY_BOARDINFO_LATE=y
>>> diff --git a/configs/evb-rk3588_defconfig b/configs/evb-rk3588_defconfig
>>> index d5f1c4b9ebc7..f49c2ca686a8 100644
>>> --- a/configs/evb-rk3588_defconfig
>>> +++ b/configs/evb-rk3588_defconfig
>>> @@ -23,6 +23,7 @@ CONFIG_FIT=y
>>> CONFIG_FIT_VERBOSE=y
>>> CONFIG_SPL_FIT_SIGNATURE=y
>>> CONFIG_SPL_LOAD_FIT=y
>>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>>> CONFIG_OF_BOARD_SETUP=y
>>> CONFIG_DEFAULT_FDT_FILE="rockchip/rk3588-evb1-v10.dtb"
>>> # CONFIG_DISPLAY_CPUINFO is not set
>>> diff --git a/configs/radxa-cm3-io-rk3566_defconfig
>>> b/configs/radxa-cm3-io-rk3566_defconfig
>>> index 56802d85cc25..488723dfaa30 100644
>>> --- a/configs/radxa-cm3-io-rk3566_defconfig
>>> +++ b/configs/radxa-cm3-io-rk3566_defconfig
>>> @@ -22,6 +22,7 @@ CONFIG_FIT=y
>>> CONFIG_FIT_VERBOSE=y
>>> CONFIG_SPL_FIT_SIGNATURE=y
>>> CONFIG_SPL_LOAD_FIT=y
>>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>>> CONFIG_DEFAULT_FDT_FILE="rockchip/rk3566-radxa-cm3-io.dtb"
>>> # CONFIG_DISPLAY_CPUINFO is not set
>>> CONFIG_DISPLAY_BOARDINFO_LATE=y
>>> diff --git a/configs/rock-3a-rk3568_defconfig
>>> b/configs/rock-3a-rk3568_defconfig
>>> index 616499f2f82b..753d03914d90 100644
>>> --- a/configs/rock-3a-rk3568_defconfig
>>> +++ b/configs/rock-3a-rk3568_defconfig
>>> @@ -27,6 +27,7 @@ CONFIG_FIT=y
>>> CONFIG_FIT_VERBOSE=y
>>> CONFIG_SPL_FIT_SIGNATURE=y
>>> CONFIG_SPL_LOAD_FIT=y
>>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>>> CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-rock-3a.dtb"
>>> # CONFIG_DISPLAY_CPUINFO is not set
>>> CONFIG_DISPLAY_BOARDINFO_LATE=y
>>> diff --git a/configs/rock-pi-4-rk3399_defconfig
>>> b/configs/rock-pi-4-rk3399_defconfig
>>> index 4b984adc6ef8..466868d80b03 100644
>>> --- a/configs/rock-pi-4-rk3399_defconfig
>>> +++ b/configs/rock-pi-4-rk3399_defconfig
>>> @@ -20,6 +20,7 @@ CONFIG_PCI=y
>>> CONFIG_DEBUG_UART=y
>>> # CONFIG_ANDROID_BOOT_IMAGE is not set
>>> CONFIG_SPL_FIT_SIGNATURE=y
>>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>>> CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-rock-pi-4a.dtb"
>>> CONFIG_DISPLAY_BOARDINFO_LATE=y
>>> CONFIG_MISC_INIT_R=y
>>> diff --git a/configs/rock5b-rk3588_defconfig
>>>
Fwd: F2-NAS2 and NSA325 contributions
Hello, I'm a hobbyist that has been playing with Linkstation LS-WVL years ago. After frying UART on that, I got Terramaster F2-NAS2 later and got u-boot from the main tree to work on it. Both landed in the closet until I dug up F2-NAS2 recently. My original source code modifications and images were nowhere to be found so I started from scratch. First tried to compile and run every defconfig available that uses Kirkwood SoC: u-boot # for config in $(grep -i kirkwood ./configs -r | cut -d ":" -f1 | cut -d "/" -f3 | sort -u); do make clean; make distclean; ARCH=arm CROSS_COMPILE=arm-none-eabi- make $config && ARCH=arm CROSS_COMPILE=arm-none-eabi- make -j64; echo $config; read; done Unfortunately, that didn't work. Then I saw the following: "...By the way this box is a close cousin of the NSA325 (Kirkwood)..." Source: https://forum.doozan.com/read.php?3,49804,49964#msg-49964 With that in mind, I've found the first compiled version I could lay my hands on: https://forum.doozan.com/read.php?3,12381 Actual file: https://bitly.com/2xnfGmv The file worked, here is the source: https://github.com/mibodhi/u-boot-kirkwood The thing didn't compile and threw errors about libfdt: https://github.com/hardkernel/u-boot/issues/73 This is when I started porting it to the main u-boot tree from that fork. I got it to boot on Terramaster F2-NAS2. To continue, I have 2 questions: 1) Original changes ported to the original tree were for NSA325. Can somebody test them on their own box or that's not something you would do? 2) I have changes for Terramaster F2-NAS2 derived from #1 as well. How do I submit them since the PR pointed me to the mailing list: https://github.com/u-boot/u-boot/pull/326 Please do not look at the changes in the PR, they are old and irrelevant. I will close or update it, depending on what I hear back. Best, - Henry
Re: [PATCH] smegw01: Fix wrong symbol override
On Tue, Jun 27, 2023 at 01:57:49PM -0300, Fabio Estevam wrote: > From: Eduard Strehlau > > board_mmc_get_env_part() is not called as the default implementation > of mmc_get_env_part() is used. > > Fix this problem by directly calling mmc_get_env_part() instead. > > Signed-off-by: Eduard Strehlau > Signed-off-by: Fabio Estevam Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] CI: Azure: Split keymile jobs out
On Mon, Jun 26, 2023 at 03:19:54PM -0400, Tom Rini wrote: > Currently the PowerPC build job in Azure will hit the maximum time limit > for a build and stop. Looking at the job, the easiest path to reducing > it is to move Keymile vendor boards to their own job and exclude them > from the PowerPC one (and while at this, the ls102 job). > > Signed-off-by: Tom Rini > Reviewed-by: Heiko Schocher Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] board: freescale: imx93_evk: Fix MMC environment offset boot conflict.
On Tue, Apr 11, 2023 at 04:05:31PM -0400, Ken Sloat wrote: > Currently, the imx93_evk is configured with CONFIG_ENV_IS_IN_MMC and the > chosen environment offset in the config is 0x40. Unless the user > programs the associated fuses, this offset is the default secondary boot > image offset used by the i.MX 93 ROM bootloader. With certain > combinations of environmental variables, the CRC and beginning of the > environment can potentially falsely appear as a valid boot image > container header. If the expected "sw_version" offset within this > mistaken boot image container is greater than the primary's, the ROM > bootloader can skip booting of the primary image altogether and attempt > to boot with the content of the environment data. This will then hang > the system. > > To fix this, move the environment from 0x40 to 0x70 reserving up > to 3 MB at 0x40 for any actual secondary user image container. > > Signed-off-by: Ken Sloat > Reviewed-by: Peng Fan > Reviewed-by: Fabio Estevam Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: Pull request: u-boot-rockchip-20230629
On Thu, Jun 29, 2023 at 08:13:42PM +0800, Kever Yang wrote: > Hi Tom, > > > Please pull the fixex for rockchip platform: > - rockchip inno phy fix; > - pinctrl driver in SPL arort in specific case; > - fix IO port voltage for rock5b-rk3588 board; > > CI: > https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/pipelines/16732 > > Thanks, > - Kever > > The following changes since commit 580eb31199be8a822e62f20965854a242f895d03: > > Merge branch 'riscv-fixes' of > https://source.denx.de/u-boot/custodians/u-boot-riscv (2023-06-27 09:39:58 > -0400) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-rockchip.git > tags/u-boot-rockchip-20230629 > > for you to fetch changes up to d77d5301d7dc333db0425ee82499fe362abd259d: > > board: rockchip: rock5b-rk3588: fix description (2023-06-29 18:43:42 +0800) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: v2023.07-rc5 regression: Image overlaps SPL
On Thu, Jun 29, 2023 at 04:19:22PM +0200, Francesco Dolcini wrote:
> Hello Marek,
> as briefly discussed off-list it looks like
> commit 77aed22b48ab ("spl: spl_legacy: Add extra address checks") introduces a
> regression on some board/arch, at least colibri and apalis imx6 fails to boot
> now
>
> ```
> Trying to boot from MMC1
> SPL: Image overlaps SPL
> resetting ...
> ```
>
> From a quick check verdin imx8mm and imx8mp are not affected.
>
> I also noticed something weird on a colibri imx7s, this is not using SPL,
> likely something completly different, however given this is new also from rc5
> I
> thought it's valuable to report:
>
> ```
> U-Boot 2023.07-rc5-0.0.0-devel+git.580eb31199be (Jun 27 2023 - 13:39:58 +)
> CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz)
> CPU: Extended Commercial temperature grade (-20C to 105C) at 30C
> Reset cause: POR
> DRAM: initcall sequence 8786eafc failed at call 8781b351 (err=-3)
> ### ERROR ### Please RESET the board ###
> ```
>
> Currently at the EOSS in Prague, so no way to do more testing/debugging, but I
> tought it was better to start reporting this on the list since we are getting
> close to the actual release.
Well ugh. And my mx6cuboxi was out of my CI loop so I didn't see this
at the time. Does reverting just the legacy checks patch fix the i.MX7
case as well?
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] doc: imx: habv4: Fix typo in 'signing'
On Thu, Jun 29, 2023 at 12:26 PM Fabio Estevam wrote: > > From: Fabio Estevam > > Fix two occurrences where 'signing' is misspelled. > > Signed-off-by: Fabio Estevam > --- > doc/imx/habv4/guides/mx6_mx7_secure_boot.txt | 2 +- > doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/doc/imx/habv4/guides/mx6_mx7_secure_boot.txt > b/doc/imx/habv4/guides/mx6_mx7_secure_boot.txt > index 53f71fbc3e2b..7fba84a39479 100644 > --- a/doc/imx/habv4/guides/mx6_mx7_secure_boot.txt > +++ b/doc/imx/habv4/guides/mx6_mx7_secure_boot.txt > @@ -113,7 +113,7 @@ the U-Boot build, the example below is a log for > mx7dsabresd_defconfig target: > 1.4 Signing the U-Boot binary > -- > > -The CST tool is used for singing the U-Boot binary and generating a CSF > binary, > +The CST tool is used for signing the U-Boot binary and generating a CSF > binary, > users should input the CSF description file created in the step above and > should receive a CSF binary, which contains the CSF commands, SRK table, > signatures and certificates. > diff --git a/doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt > b/doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt > index fde0f27efdc5..56b8cd62cb64 100644 > --- a/doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt > +++ b/doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt > @@ -145,7 +145,7 @@ addresses, the csf_uboot.txt can be used as example: > 1.4 Signing the images > --- > > -The CST tool is used for singing the U-Boot binary and generating a CSF > binary, > +The CST tool is used for signing the U-Boot binary and generating a CSF > binary, > users should input the CSF description file created in the step above and > receive a CSF binary, which contains the CSF commands, SRK table, signatures > and certificates. > -- > 2.34.1 > Reviewed-by: Tim Harvey Best regards, Tim
[PATCH] doc: imx: habv4: Fix typo in 'signing'
From: Fabio Estevam Fix two occurrences where 'signing' is misspelled. Signed-off-by: Fabio Estevam --- doc/imx/habv4/guides/mx6_mx7_secure_boot.txt | 2 +- doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/imx/habv4/guides/mx6_mx7_secure_boot.txt b/doc/imx/habv4/guides/mx6_mx7_secure_boot.txt index 53f71fbc3e2b..7fba84a39479 100644 --- a/doc/imx/habv4/guides/mx6_mx7_secure_boot.txt +++ b/doc/imx/habv4/guides/mx6_mx7_secure_boot.txt @@ -113,7 +113,7 @@ the U-Boot build, the example below is a log for mx7dsabresd_defconfig target: 1.4 Signing the U-Boot binary -- -The CST tool is used for singing the U-Boot binary and generating a CSF binary, +The CST tool is used for signing the U-Boot binary and generating a CSF binary, users should input the CSF description file created in the step above and should receive a CSF binary, which contains the CSF commands, SRK table, signatures and certificates. diff --git a/doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt b/doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt index fde0f27efdc5..56b8cd62cb64 100644 --- a/doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt +++ b/doc/imx/habv4/guides/mx6_mx7_spl_secure_boot.txt @@ -145,7 +145,7 @@ addresses, the csf_uboot.txt can be used as example: 1.4 Signing the images --- -The CST tool is used for singing the U-Boot binary and generating a CSF binary, +The CST tool is used for signing the U-Boot binary and generating a CSF binary, users should input the CSF description file created in the step above and receive a CSF binary, which contains the CSF commands, SRK table, signatures and certificates. -- 2.34.1
Re: [PATCH 07/10] test: dm: add SCMI base protocol test
Hi AKASHI,
On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
wrote:
>
> Added is a new unit test for SCMI base protocol, which will exercise all
> the commands provided by the protocol, except SCMI_BASE_NOTIFY_ERRORS.
> $ ut dm scmi_base
> It is assumed that test.dtb is used as sandbox's device tree.
>
> Signed-off-by: AKASHI Takahiro
> ---
> test/dm/scmi.c | 112 +
> 1 file changed, 112 insertions(+)
>
> diff --git a/test/dm/scmi.c b/test/dm/scmi.c
> index 881be3171b7c..563017bb63e0 100644
> --- a/test/dm/scmi.c
> +++ b/test/dm/scmi.c
> @@ -16,6 +16,9 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
> #include
> #include
> #include
> @@ -95,6 +98,115 @@ static int dm_test_scmi_sandbox_agent(struct
> unit_test_state *uts)
> }
> DM_TEST(dm_test_scmi_sandbox_agent, UT_TESTF_SCAN_FDT);
>
> +static int dm_test_scmi_base(struct unit_test_state *uts)
> +{
> + struct udevice *agent_dev, *base;
> + struct scmi_agent_priv *priv;
> + const struct scmi_base_ops *ops;
> + u32 version, num_agents, num_protocols, impl_version;
> + u32 attributes, agent_id;
> + char vendor[SCMI_BASE_NAME_LENGTH_MAX],
> +agent_name[SCMI_BASE_NAME_LENGTH_MAX];
> + u8 *protocols;
> + int ret;
> +
> + /* preparation */
> + ut_assertok(uclass_get_device_by_name(UCLASS_SCMI_AGENT, "scmi",
> + _dev));
> + ut_assertnonnull(agent_dev);
> + ut_assertnonnull(priv = dev_get_uclass_plat(agent_dev));
> + ut_assertnonnull(base = scmi_get_protocol(agent_dev,
> + SCMI_PROTOCOL_ID_BASE));
> + ut_assertnonnull(ops = dev_get_driver_ops(base));
> +
> + /* version */
> + ret = (*ops->protocol_version)(base, );
Can you add uclass helpers to call each of the methods? That is how it
is commonly done. You should not be calling ops->xxx directly here.
> + ut_assertok(ret);
> + ut_asserteq(priv->version, version);
> +
> + /* protocol attributes */
> + ret = (*ops->protocol_attrs)(base, _agents, _protocols);
> + ut_assertok(ret);
> + ut_asserteq(priv->num_agents, num_agents);
> + ut_asserteq(priv->num_protocols, num_protocols);
> +
> + /* discover vendor */
> + ret = (*ops->base_discover_vendor)(base, vendor);
> + ut_assertok(ret);
> + ut_asserteq_str(priv->vendor, vendor);
> +
> + /* message attributes */
> + ret = (*ops->protocol_message_attrs)(base,
> +SCMI_BASE_DISCOVER_SUB_VENDOR,
> +);
> + ut_assertok(ret);
> + ut_assertok(attributes);
> +
> + /* discover sub vendor */
> + ret = (*ops->base_discover_sub_vendor)(base, vendor);
> + ut_assertok(ret);
> + ut_asserteq_str(priv->sub_vendor, vendor);
> +
> + /* impl version */
> + ret = (*ops->base_discover_impl_version)(base, _version);
> + ut_assertok(ret);
> + ut_asserteq(priv->impl_version, impl_version);
> +
> + /* discover agent (my self) */
> + ret = (*ops->base_discover_agent)(base, 0x,
> + _id, agent_name);
> + ut_assertok(ret);
> + ut_asserteq(priv->agent_id, agent_id);
> + ut_asserteq_str(priv->agent_name, agent_name);
> +
> + /* discover protocols */
> + ret = (*ops->base_discover_list_protocols)(base, );
> + ut_asserteq(num_protocols, ret);
> + ut_asserteq_mem(priv->protocols, protocols, sizeof(u8) *
> num_protocols);
> + free(protocols);
> +
> + /*
> +* NOTE: Sandbox SCMI driver handles device-0 only. It supports
> setting
> +* access and protocol permissions, but doesn't allow unsetting them
> nor
> +* resetting the configurations.
> +*/
> + /* set device permissions */
> + ret = (*ops->base_set_device_permissions)(base, agent_id, 0,
> +
> SCMI_BASE_SET_DEVICE_PERMISSIONS_ACCESS);
> + ut_assertok(ret); /* SCMI_SUCCESS */
> + ret = (*ops->base_set_device_permissions)(base, agent_id, 1,
> +
> SCMI_BASE_SET_DEVICE_PERMISSIONS_ACCESS);
> + ut_asserteq(-ENOENT, ret); /* SCMI_NOT_FOUND */
> + ret = (*ops->base_set_device_permissions)(base, agent_id, 0, 0);
> + ut_asserteq(-EACCES, ret); /* SCMI_DENIED */
> +
> + /* set protocol permissions */
> + ret = (*ops->base_set_protocol_permissions)(base, agent_id, 0,
> + SCMI_PROTOCOL_ID_CLOCK,
> +
> SCMI_BASE_SET_PROTOCOL_PERMISSIONS_ACCESS);
> + ut_assertok(ret); /* SCMI_SUCCESS */
> + ret = (*ops->base_set_protocol_permissions)(base, agent_id, 1,
>
Re: [PATCH 09/10] doc: cmd: add documentation for scmi
Hi AKASHI, On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro wrote: > > This is a help text for scmi command. > > Signed-off-by: AKASHI Takahiro > --- > doc/usage/cmd/scmi.rst | 98 ++ > 1 file changed, 98 insertions(+) > create mode 100644 doc/usage/cmd/scmi.rst > > diff --git a/doc/usage/cmd/scmi.rst b/doc/usage/cmd/scmi.rst > new file mode 100644 > index ..20cdae4b877d > --- /dev/null > +++ b/doc/usage/cmd/scmi.rst > @@ -0,0 +1,98 @@ > +.. SPDX-License-Identifier: GPL-2.0+: > + > +scmi command > + > + > +Synopsis > + > + > +:: > + > +scmi base info > +scmi base perm_dev > +scmi base perm_proto > +scmi base reset > + > +Description > +--- > + > +The scmi command is used to access and operate on SCMI server. > + > +scmi base info > +~~ > +Show base information about SCMI server and supported protocols > + > +scmi base perm_dev > +~~ > +Allow or deny access permission to the device > + > +scmi base perm_proto > + > +Allow or deny access to the protocol on the device > + > +scmi base reset > +~~~ > +Reset the existing configurations > + > +Parameters are used as follows: > + > + > +Agent ID what is this? > + > + > +Device ID what is this? > + > + > +Protocol ID, should not be 0x10 (base protocol) what is this? Please add more detail > + > + > +Flags to control the action. See SCMI specification for > +defined values. ? Please add the flags here, or at the very least provide a URL and page number, etc. > + > +Example > +--- > + > +Obtain basic information about SCMI server: > + > +:: > + > +=> scmi base info > +SCMI device: scmi > + protocol version: 0x2 > + # of agents: 3 > + 0: platform > +> 1: OSPM > + 2: PSCI > + # of protocols: 4 > + Power domain management > + Performance domain management > + Clock management > + Sensor management > + vendor: Linaro > + sub vendor: PMWG > + impl version: 0x20b > + > +Ask for access permission to device#0: > + > +:: > + > +=> scmi base perm_dev 1 0 1 > + > +Reset configurations with all access permission settings retained: > + > +:: > + > +=> scmi base reset 1 0 > + > +Configuration > +- > + > +The scmi command is only available if CONFIG_CMD_SCMI=y. > +Default n because this command is mainly for debug purpose. > + > +Return value > + > + > +The return value ($?) is set to 0 if the operation succeeded, > +1 if the operation failed or -1 if the operation failed due to > +a syntax error. > -- > 2.41.0 > Regards, Simon
Re: [PATCH v2 2/4] cmd: Add a sysinfo command
Hi Detlev,
On Wed, 28 Jun 2023 at 20:33, Detlev Casanova
wrote:
>
> The command is able to show different information for the running
> system:
> * Model name
> * Board ID
> * Revision
>
> This command can be used by boot shell scripts to select configurations
> depending on the specific running system.
>
> Signed-off-by: Detlev Casanova
> ---
> cmd/Kconfig | 6 +++
> cmd/Makefile | 1 +
> cmd/sysinfo.c | 134 ++
> 3 files changed, 141 insertions(+)
> create mode 100644 cmd/sysinfo.c
Please add a test - see test/dm/sysinfo.c
Please also add doc/usage/cmd/sysinfo.rst
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 02e54f1e50f..9fb778ce809 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -210,6 +210,12 @@ config CMD_SBI
> help
> Display information about the SBI implementation.
>
> +config CMD_SYSINFO
> + bool "sysinfo"
> + depends on SYSINFO
> + help
> + Display information about the system.
> +
> endmenu
>
> menu "Boot commands"
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 6c37521b4e2..ba4d6de9a1b 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -165,6 +165,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
> obj-$(CONFIG_CMD_STRINGS) += strings.o
> obj-$(CONFIG_CMD_SMC) += smccc.o
> obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o
> +obj-$(CONFIG_CMD_SYSINFO) += sysinfo.o
> obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
> obj-$(CONFIG_CMD_TEMPERATURE) += temperature.o
> obj-$(CONFIG_CMD_TERMINAL) += terminal.o
> diff --git a/cmd/sysinfo.c b/cmd/sysinfo.c
> new file mode 100644
> index 000..513ea0416a2
> --- /dev/null
> +++ b/cmd/sysinfo.c
> @@ -0,0 +1,134 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2023
> + * Detlev Casanova
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +static int get_sysinfo(struct udevice **dev)
Please use devp since this is the common convention (indicating it is
a return value)
> +{
> + int ret = sysinfo_get(dev);
> +
> + if (ret) {
> + debug("Cannot get sysinfo: %d\n", ret);
printf() to be more helpful?
> + return ret;
> + }
> +
> + ret = sysinfo_detect(*dev);
> + if (ret) {
> + debug("Cannot detect sysinfo: %d\n", ret);
printf() to be more helpful?
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int do_sysinfo_model(struct cmd_tbl *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + struct udevice *dev;
> + char model[64];
> + int ret = get_sysinfo();
> +
> + if (ret)
> + return CMD_RET_FAILURE;
> +
> + ret = sysinfo_get_str(dev,
> + SYSINFO_ID_BOARD_MODEL,
> + sizeof(model),
> + model);
> +
> + if (ret) {
> + debug("Cannot get sysinfo str: %d\n", ret);
> + return CMD_RET_FAILURE;
> + }
> +
> + if (argc == 2)
> + env_set(argv[1], model);
check return value
same below
> + else
> + printf("%s\n", model);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_sysinfo_id(struct cmd_tbl *cmdtp, int flag, int argc,
> +char *const argv[])
> +{
> + struct udevice *dev;
> + u32 board_id;
> + int ret = get_sysinfo();
> +
> + if (ret)
> + return CMD_RET_FAILURE;
> +
> + ret = sysinfo_get_int(dev,
> + SYSINFO_ID_BOARD_ID,
> + _id);
can you fit on one line?
> +
> + if (ret) {
> + debug("Cannot get sysinfo int: %d\n", ret);
> + return CMD_RET_FAILURE;
> + }
> +
> + if (argc == 2)
> + env_set_hex(argv[1], board_id);
> + else
> + printf("0x%02x\n", board_id);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_sysinfo_revision(struct cmd_tbl *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + struct udevice *dev;
> + int rev_major;
> + int rev_minor;
> + char rev[4];
> + int ret = get_sysinfo();
> +
> + if (ret)
> + return CMD_RET_FAILURE;
> +
> + ret = sysinfo_get_int(dev,
> + SYSINFO_ID_BOARD_REVISION_MAJOR,
> + _major);
> +
> + if (ret) {
> + debug("Cannot get sysinfo int: %d\n", ret);
> + return CMD_RET_FAILURE;
> + }
> +
> + ret = sysinfo_get_int(dev,
> + SYSINFO_ID_BOARD_REVISION_MINOR,
> + _minor);
> +
drop blank line between ret = and if (ret) ...please fix globally
> + if (ret) {
> + debug("Cannot get sysinfo int: %d\n", ret);
> + return CMD_RET_FAILURE;
> +
Re: [PATCH 01/12] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon
On Thu, 29 Jun 2023 at 15:59, wrote: > > From: Lukas Funke > > Check if elf tools are available when running DecodeElf(). Also > remove superfuous semicolon at line ending. > > Signed-off-by: Lukas Funke > --- > > tools/binman/elf.py | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass
Re: [PATCHv2 1/3] net/lwip: add lwip-external submodule
Hi Maxim, On Thu, 29 Jun 2023 at 13:39, Maxim Uvarov wrote: > > This commit adds the lwip library as a git submodule. I think > there has to be advantages to compile lwip inside U-boot, > i.e. use the same compiler and flags as the main code. > One of them is LTO and the other is to enable additional debug > options for network protocol during development. Also we can > copy lwip library code inside U-boot, but for now I don't want > to send all lwip code to the mailing list. So it's git submodule. Fairy nuff. You can send the full patch later. Regards, Simon
Re: [PATCH v2 1/4] sysinfo: Add IDs for board id and revision
Hi Detlev,
On Wed, 28 Jun 2023 at 20:32, Detlev Casanova
wrote:
>
> These IDs will be used by the sysinfo command. The new IDs are:
> * SYSINFO_ID_BOARD_ID: The board ID as an integer
> * SYSINFO_ID_BOARD_REVISION: The board revision as a string
>
> Signed-off-by: Detlev Casanova
> ---
> include/sysinfo.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/sysinfo.h b/include/sysinfo.h
> index b140d742e93..43d9e91212d 100644
> --- a/include/sysinfo.h
> +++ b/include/sysinfo.h
> @@ -47,6 +47,11 @@ enum sysinfo_id {
> /* For show_board_info() */
> SYSINFO_ID_BOARD_MODEL,
>
> + /* For sysinfo command */
> + SYSINFO_ID_BOARD_ID,
> + SYSINFO_ID_BOARD_REVISION_MAJOR,
> + SYSINFO_ID_BOARD_REVISION_MINOR,
Please mention that these are strings. Are they?
Also how about REV instead of REVISION as it is shorter
> +
> /* First value available for downstream/board used */
> SYSINFO_ID_USER = 0x1000,
> };
> --
> 2.39.3
>
Regards,
Simon
Re: [PATCH] bootstd: USB devtype detection for script boot
Hi John,
On Thu, 29 Jun 2023 at 01:03, John Clark wrote:
>
> > I only just thought of this, but I think it is better to check
> blk->uclass_id == UCLASS_USB instead, since it avoids a string comparison.
>
> I am not finding anything as direct as your pseudo code above suggests.
> The closest I can find is a compare like this:
>
> UCLASS_MASS_STORAGE == device_get_uclass_id(dev_get_parent(bflow->blk))
>
> Which has the overhead of calling two new functions that otherwise would
> not be called and could match other mass storage devices other than USB
> (am I right to assume this?).
>
> Can you shed additional insight into "blk->uclass_id"?
Oh sorry, I mean the blk_desc structure attached to the block device.
struct blk_desc *desc = dev_get_uclass_plat(blk);
Then desc->uclass_id is what you want. It is the uclass of the media
device (typically the parent of the block device).
>
> blk is a udevice without a uclass_id member:
>
> ./dm/device.h
>
> struct udevice {
> const struct driver *driver;
> const char *name;
> void *plat_;
> void *parent_plat_;
> void *uclass_plat_;
> ulong driver_data;
> struct udevice *parent;
> void *priv_;
> struct uclass *uclass;
> void *uclass_priv_;
> void *parent_priv_;
> struct list_head uclass_node;
> struct list_head child_head;
> struct list_head sibling_node;
> #if !CONFIG_IS_ENABLED(OF_PLATDATA_RT)
> u32 flags_;
> #endif
> int seq_;
> #if CONFIG_IS_ENABLED(OF_REAL)
> ofnode node_;
> #endif
> #if CONFIG_IS_ENABLED(DEVRES)
> struct list_head devres_head;
> #endif
> #if CONFIG_IS_ENABLED(DM_DMA)
> ulong dma_offset;
> #endif
> #if CONFIG_IS_ENABLED(IOMMU)
> struct udevice *iommu;
> #endif
> };
You can use device_get_uclass_id(blk) to get the uclass ID of the
block device, but of course it will just be UCLASS_BLK.
>
>
> I am not a prolific open source contributor, so I am not sure if a
> side-bar conversation like this is one we should be having with the
> whole list? Something this trivial seems like it would just be noise to
> include hundreds of people on.
>
> I appreciate your advice here, and thanks for your time.
You can just copy the list on all emails. I've done it here.
Regards,
Simon
>
>
> John Clark
>
>
> On 6/28/23 3:42 AM, Simon Glass wrote:
> > Hi John,
> >
> > On Tue, 27 Jun 2023 at 15:39, John Clark wrote:
> >> Change the device type from "usb_mass_storage" to "usb" when
> >> booting a script.
> >>
> >> Before this change:
> >>=> printenv devtype
> >>devtype=usb_mass_storage
> >>
> >> After this change:
> >>=> printenv devtype
> >>devtype=usb
> >>
> >> Signed-off-by: John Clark
> >> ---
> >>
> >> boot/bootmeth_script.c | 8 ++--
> >> 1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
> >> index 225eb18ee6..9fdadb3005 100644
> >> --- a/boot/bootmeth_script.c
> >> +++ b/boot/bootmeth_script.c
> >> @@ -187,10 +187,14 @@ static int script_set_bootflow(struct udevice *dev,
> >> struct bootflow *bflow,
> >> static int script_boot(struct udevice *dev, struct bootflow *bflow)
> >> {
> >> struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
> >> + const char *devtype = blk_get_devtype(bflow->blk);
> >> ulong addr;
> >> int ret;
> >>
> >> - ret = env_set("devtype", blk_get_devtype(bflow->blk));
> >> + if (!strcmp("usb_mass_storage", devtype))
> > I only just thought of this, but I think it is better to check
> > blk->uclass_id == UCLASS_USB instead, since it avoids a string
> > comparison.
> >
> >> + ret = env_set("devtype", "usb");
> >> + else
> >> + ret = env_set("devtype", devtype);
> >> if (!ret)
> >> ret = env_set_hex("devnum", desc->devnum);
> >> if (!ret)
> >> @@ -198,7 +202,7 @@ static int script_boot(struct udevice *dev, struct
> >> bootflow *bflow)
> >> if (!ret)
> >> ret = env_set("prefix", bflow->subdir);
> >> if (!ret && IS_ENABLED(CONFIG_ARCH_SUNXI) &&
> >> - !strcmp("mmc", blk_get_devtype(bflow->blk)))
> >> + !strcmp("mmc", devtype))
> >> ret = env_set_hex("mmc_bootdev", desc->devnum);
> >> if (ret)
> >> return log_msg_ret("env", ret);
> >> --
> >> 2.39.2
> >>
> > Regards,
> > Simon
Re: [PATCH 05/10] firmware: scmi: fake base protocol commands on sandbox
On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro wrote: > > This is a simple implementation of SCMI base protocol for sandbox. > The main use is in SCMI unit test. > > Signed-off-by: AKASHI Takahiro > --- > drivers/firmware/scmi/sandbox-scmi_agent.c | 359 - > 1 file changed, 358 insertions(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH 03/12] binman: Don't decompress data while signing
Hi, On Thu, 29 Jun 2023 at 15:59, wrote: > > From: Lukas Funke > > While signing a fit compressed data (i.e. 'blob-ext') is decompressed, > but never compressed again. When compressed data was wrapped in a > section, decompression leads to an error because the outer section had > the original compressed size but the inner entry has the > uncompressed size now. > > While singing there is no reason to decompress data. Thus, decompression signing ? > should be disabled. > > Furthermore, bintools should be collected before loading the data. This > way bintools are available if processing is required on a node. > > Signed-off-by: Lukas Funke > --- > > tools/binman/control.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass > > diff --git a/tools/binman/control.py b/tools/binman/control.py > index 68597c4e77..affc33ff3d 100644 > --- a/tools/binman/control.py > +++ b/tools/binman/control.py > @@ -306,8 +306,8 @@ def BeforeReplace(image, allow_resize): > image: Image to prepare > """ > state.PrepareFromLoadedData(image) > -image.LoadData() > image.CollectBintools() > +image.LoadData(decomp=False) > > # If repacking, drop the old offset/size values except for the original > # ones, so we are only left with the constraints. > -- > 2.30.2 >
Re: [PATCH 2/3] binman: Allow cipher node as special section
On Tue, 27 Jun 2023 at 08:39, wrote: > > From: Christian Taedcke > > The new encrypted etype generates a cipher node in the device tree > that should not be evaluated by binman, but still be kept in the > output device tree. > > Signed-off-by: Christian Taedcke > --- > > tools/binman/etype/section.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH 08/10] cmd: add scmi command for SCMI firmware
Hi AKASHI,
On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
wrote:
>
> This command, "scmi", provides a command line interface to various SCMI
> protocols. It supports at least initially SCMI base protocol with the sub
> command, "base", and is intended mainly for debug purpose.
>
> Signed-off-by: AKASHI Takahiro
> ---
> cmd/Kconfig | 8 ++
> cmd/Makefile | 1 +
> cmd/scmi.c | 373 +++
> 3 files changed, 382 insertions(+)
> create mode 100644 cmd/scmi.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 02e54f1e50fe..065470a76295 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2504,6 +2504,14 @@ config CMD_CROS_EC
> a number of sub-commands for performing EC tasks such as
> updating its flash, accessing a small saved context area
> and talking to the I2C bus behind the EC (if there is one).
> +
> +config CMD_SCMI
> + bool "Enable scmi command"
> + depends on SCMI_FIRMWARE
> + default n
> + help
> + This command provides user interfaces to several SCMI protocols,
> + including Base protocol.
Please mention what is SCMI
> endmenu
>
> menu "Filesystem commands"
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 6c37521b4e2b..826e0b74b587 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -156,6 +156,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
> obj-$(CONFIG_CMD_NVME) += nvme.o
> obj-$(CONFIG_SANDBOX) += sb.o
> obj-$(CONFIG_CMD_SF) += sf.o
> +obj-$(CONFIG_CMD_SCMI) += scmi.o
> obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
> obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
> obj-$(CONFIG_CMD_SEAMA) += seama.o
> diff --git a/cmd/scmi.c b/cmd/scmi.c
> new file mode 100644
> index ..c97f77e97d95
> --- /dev/null
> +++ b/cmd/scmi.c
> @@ -0,0 +1,373 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * SCMI utility command
> + *
> + * Copyright (c) 2023 Linaro Limited
> + * Author: AKASHI Takahiro
> + */
> +
> +#include
> +#include
> +#include
> +#include /* uclass_get_device */
Goes before linux/bitfield.h
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static struct udevice *agent;
> +static struct udevice *base_proto;
> +static const struct scmi_base_ops *ops;
> +
> +struct {
> + enum scmi_std_protocol id;
> + const char *name;
> +} protocol_name[] = {
> + {SCMI_PROTOCOL_ID_BASE, "Base"},
> + {SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
> + {SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
> + {SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
> + {SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
> + {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
> + {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
> + {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"},
> +};
> +
> +/**
> + * scmi_convert_id_to_string() - get the name of SCMI protocol
> + *
> + * @id:Protocol ID
> + *
> + * Get the printable name of the protocol, @id
> + *
> + * Return: Name string on success, NULL on failure
> + */
> +static const char *scmi_convert_id_to_string(enum scmi_std_protocol id)
scmi_lookup_id?
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(protocol_name); i++)
> + if (id == protocol_name[i].id)
> + return protocol_name[i].name;
> +
> + return NULL;
> +}
> +
> +/**
> + * do_scmi_base_info() - get the information of SCMI services
> + *
> + * @cmdtp: Command table
> + * @flag: Command flag
> + * @argc: Number of arguments
> + * @argv: Argument array
> + *
> + * Get the information of SCMI services using various interfaces
> + * provided by the Base protocol.
> + *
> + * Return: CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure
> + */
> +static int do_scmi_base_info(struct cmd_tbl *cmdtp, int flag, int argc,
> +char * const argv[])
> +{
> + u32 agent_id, num_protocols;
> + u8 agent_name[SCMI_BASE_NAME_LENGTH_MAX], *protocols;
> + int i, ret;
> +
> + if (argc != 1)
> + return CMD_RET_USAGE;
> +
> + printf("SCMI device: %s\n", agent->name);
> + printf(" protocol version: 0x%x\n", scmi_version(agent));
> + printf(" # of agents: %d\n", scmi_num_agents(agent));
> + for (i = 0; i < scmi_num_agents(agent); i++) {
> + ret = (*ops->base_discover_agent)(base_proto, i, _id,
> + agent_name);
> + if (ret) {
> + if (ret != -EOPNOTSUPP)
> + printf("base_discover_agent() failed for id:
> %d (%d)\n",
> + i, ret);
> + break;
> + }
> + printf("%c%2d: %s\n", i == scmi_agent_id(agent) ? '>' : '
> ',
> + i, agent_name);
> +
Re: [PATCH 01/10] firmware: scmi: implement SCMI base protocol
Hi AKASHI,
On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro
wrote:
>
> SCMI base protocol is mandatory according to the SCMI specification.
>
> With this patch, SCMI base protocol can be accessed via SCMI transport
> layers. All the commands, except SCMI_BASE_NOTIFY_ERRORS, are supported.
> This is because U-Boot doesn't support interrupts and the current transport
> layers are not able to handle asynchronous messages properly.
>
> Signed-off-by: AKASHI Takahiro
> ---
> drivers/firmware/scmi/Makefile | 1 +
> drivers/firmware/scmi/base.c | 517 +
> include/dm/uclass-id.h | 1 +
> include/scmi_protocols.h | 201 -
> 4 files changed, 718 insertions(+), 2 deletions(-)
> create mode 100644 drivers/firmware/scmi/base.c
>
> diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
> index b2ff483c75a1..1a23d4981709 100644
> --- a/drivers/firmware/scmi/Makefile
> +++ b/drivers/firmware/scmi/Makefile
> @@ -1,4 +1,5 @@
> obj-y += scmi_agent-uclass.o
> +obj-y += base.o
> obj-y += smt.o
> obj-$(CONFIG_SCMI_AGENT_SMCCC) += smccc_agent.o
> obj-$(CONFIG_SCMI_AGENT_MAILBOX) += mailbox_agent.o
[..]
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 307ad6931ca7..f7a110852321 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -116,6 +116,7 @@ enum uclass_id {
> UCLASS_RNG, /* Random Number Generator */
> UCLASS_RTC, /* Real time clock device */
> UCLASS_SCMI_AGENT, /* Interface with an SCMI server */
> + UCLASS_SCMI_BASE, /* Interface for SCMI Base protocol */
> UCLASS_SCSI,/* SCSI device */
> UCLASS_SERIAL, /* Serial UART */
> UCLASS_SIMPLE_BUS, /* Bus with child devices */
> diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
> index a220cb2a91ad..769041654534 100644
> --- a/include/scmi_protocols.h
> +++ b/include/scmi_protocols.h
> @@ -15,6 +15,8 @@
> * https://developer.arm.com/docs/den0056/b
> */
>
> +#define SCMI_BASE_NAME_LENGTH_MAX 16
> +
> enum scmi_std_protocol {
> SCMI_PROTOCOL_ID_BASE = 0x10,
> SCMI_PROTOCOL_ID_POWER_DOMAIN = 0x11,
> @@ -41,12 +43,207 @@ enum scmi_status_code {
> };
>
> /*
> - * Generic message IDs
> + * SCMI Base Protocol
> */
> -enum scmi_discovery_id {
> +#define SCMI_BASE_PROTOCOL_VERSION 0x2
> +
> +enum scmi_base_message_id {
> SCMI_PROTOCOL_VERSION = 0x0,
> SCMI_PROTOCOL_ATTRIBUTES = 0x1,
> SCMI_PROTOCOL_MESSAGE_ATTRIBUTES = 0x2,
> + SCMI_BASE_DISCOVER_VENDOR = 0x3,
> + SCMI_BASE_DISCOVER_SUB_VENDOR = 0x4,
> + SCMI_BASE_DISCOVER_IMPL_VERSION = 0x5,
> + SCMI_BASE_DISCOVER_LIST_PROTOCOLS = 0x6,
> + SCMI_BASE_DISCOVER_AGENT = 0x7,
> + SCMI_BASE_NOTIFY_ERRORS = 0x8,
> + SCMI_BASE_SET_DEVICE_PERMISSIONS = 0x9,
> + SCMI_BASE_SET_PROTOCOL_PERMISSIONS = 0xa,
> + SCMI_BASE_RESET_AGENT_CONFIGURATION = 0xb,
> +};
> +
> +/**
> + * struct scmi_protocol_version_out - Response for SCMI_PROTOCOL_VERSION
> + * command
> + * @status:SCMI command status
> + * @version: Protocol version
> + */
> +struct scmi_protocol_version_out {
> + s32 status;
> + u32 version;
> +};
> +
> +/**
> + * struct scmi_protocol_attrs_out - Response for SCMI_PROTOCOL_ATTRIBUTES
> + * command
> + * @status:SCMI command status
> + * @attributes:Protocol attributes or implementation details
> + */
> +struct scmi_protocol_attrs_out {
> + s32 status;
> + u32 attributes;
> +};
> +
> +#define SCMI_PROTOCOL_ATTRS_NUM_AGENTS(attributes) \
> + (((attributes) & GENMASK(15, 8)) >> 8)
> +#define SCMI_PROTOCOL_ATTRS_NUM_PROTOCOLS(attributes) \
> + ((attributes) & GENMASK(7, 0))
> +
> +/**
> + * struct scmi_protocol_msg_attrs_out - Response for
> + * SCMI_PROTOCOL_MESSAGE_ATTRIBUTES
> command
> + * @status:SCMI command status
> + * @attributes:Message-specific attributes
> + */
> +struct scmi_protocol_msg_attrs_out {
> + s32 status;
> + u32 attributes;
> +};
> +
> +/**
> + * struct scmi_base_discover_vendor_out - Response for
> + * SCMI_BASE_DISCOVER_VENDOR or
> + * SCMI_BASE_DISCOVER_SUB_VENDOR
> command
> + * @status:SCMI command status
> + * @vendor_identifier: Identifier of vendor or sub-vendor in string
> + */
> +struct scmi_base_discover_vendor_out {
> + s32 status;
> + u8 vendor_identifier[SCMI_BASE_NAME_LENGTH_MAX];
> +};
> +
> +/**
> + * struct scmi_base_discover_impl_version_out - Response for
> + * SCMI_BASE_DISCOVER_IMPL_VERSION
> command
> + * @status:SCMI command status
> +
Re: [PATCH 3/3] binman: Add tests for etype encrypted
On Tue, 27 Jun 2023 at 08:39, wrote: > > From: Christian Taedcke > > Add tests to reach 100% code coverage for the added etype encrypted. > > Signed-off-by: Christian Taedcke > --- > > tools/binman/ftest.py | 69 +++ > .../binman/test/282_encrypted_no_content.dts | 15 > tools/binman/test/283_encrypted_no_algo.dts | 19 + > .../test/284_encrypted_invalid_iv_file.dts| 22 ++ > tools/binman/test/285_encrypted.dts | 29 > tools/binman/test/286_encrypted_key_file.dts | 30 > .../test/287_encrypted_iv_name_hint.dts | 30 > 7 files changed, 214 insertions(+) > create mode 100644 tools/binman/test/282_encrypted_no_content.dts > create mode 100644 tools/binman/test/283_encrypted_no_algo.dts > create mode 100644 tools/binman/test/284_encrypted_invalid_iv_file.dts > create mode 100644 tools/binman/test/285_encrypted.dts > create mode 100644 tools/binman/test/286_encrypted_key_file.dts > create mode 100644 tools/binman/test/287_encrypted_iv_name_hint.dts Reviewed-by: Simon Glass
Re: [PATCH 1/3] binman: Add support for externally encrypted blobs
Hi Christian,
On Tue, 27 Jun 2023 at 08:39, wrote:
>
> From: Christian Taedcke
>
> This adds a new etype encrypted that is derived from collection.
>
> It creates a new cipher node in the related image similar to the
> cipher node used by u-boot, see boot/image-cipher.c.
> Optionally it creates a global /cipher node containing a key and iv
> using the same nameing convention as used in boot/image-cipher.c.
>
> Signed-off-by: Christian Taedcke
> ---
>
> tools/binman/etype/encrypted.py | 98 +
> 1 file changed, 98 insertions(+)
> create mode 100644 tools/binman/etype/encrypted.py
>
> diff --git a/tools/binman/etype/encrypted.py b/tools/binman/etype/encrypted.py
> new file mode 100644
> index 00..005ae56acf
> --- /dev/null
> +++ b/tools/binman/etype/encrypted.py
> @@ -0,0 +1,98 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright 2023 Weidmüller Interface GmbH & Co. KG
> +# Written by Christian Taedcke
> +#
> +# Entry-type module for cipher information of encrypted blobs/images
> +#
> +
> +from binman.etype.collection import Entry_collection
> +from dtoc import fdt_util
> +from u_boot_pylib import tools
> +
> +# This is imported if needed
> +state = None
> +
> +
> +class Entry_encrypted(Entry_collection):
> +"""Externally built encrypted binary blob
> +
> +If the file providing this blob is missing, binman can optionally ignore
> it
> +and produce a broken image with a warning.
> +
> +In addition to the inherited 'collection' for Properties / Entry
> arguments:
> +- algo: The encryption algorithm
What possible values are available? Please list them
> +- iv-name-hint: The name hint for the iv
what is the iv?
> +- key-name-hint: The name hint for the key
> +- iv-filename: The name of the file containing the iv
> +- key-filename: The name of the file containing the key
> +
> +This entry creates a cipher node in the entries' parent node (i.e. the
entry's
> +image). Optionally it also creates a cipher node in the root of the
> device
> +tree containg key and iv information.
containing
Overall I think this documentation needs to be expanded.
I wonder why this needs to be an entry type? Could the node be added
to the description by the user, instead of the entry adding it? It
feels a little strange to me, but perhaps I just need more info.
> +"""
> +
> +def __init__(self, section, etype, node):
> +# Put this here to allow entry-docs and help to work without libfdt
> +global state
> +from binman import state
> +
> +super().__init__(section, etype, node)
> +# The property key-filename is not required, because some
> implementations use keys that
> +# are not embedded in the device tree, but e.g. in the device itself
> +self.required_props = ['algo', 'key-name-hint', 'iv-filename']
> +self._algo = fdt_util.GetString(self._node, 'algo')
> +self._iv_name_hint = fdt_util.GetString(self._node, 'iv-name-hint')
> +self._key_name_hint = fdt_util.GetString(self._node, 'key-name-hint')
> +self._filename_iv = fdt_util.GetString(self._node, 'iv-filename')
> +self._filename_key = fdt_util.GetString(self._node, 'key-filename')
Here you should set these variables to None. Move the reading to ReadNode()
Sorry if there are counter-examples in the source code. But this is
the correct way.
> +
> +def ReadNode(self):
> +super().ReadNode()
> +
> +iv_filename = tools.get_input_filename(self._filename_iv)
> +self._iv = tools.read_file(iv_filename, binary=True)
Please only read the node in this method. Move file reading until
where it is needed.
> +
> +self._key = None
> +if self._filename_key:
> +key_filename = tools.get_input_filename(self._filename_key)
> +self._key = tools.read_file(key_filename, binary=True)
> +
> +def gen_entries(self):
> +super().gen_entries()
> +
> +cipher_node = state.AddSubnode(self._node.parent, "cipher")
> +cipher_node.AddString("algo", self._algo)
> +cipher_node.AddString("key-name-hint", self._key_name_hint)
> +if self._iv_name_hint:
> +cipher_node.AddString("iv-name-hint", self._iv_name_hint)
> +else:
> +cipher_node.AddData("iv", self._iv)
> +
> +if self._key or self._iv_name_hint:
> +# add cipher node in root
> +root_node = self._node.parent.parent.parent
The root node is self.GetImage()._node
But why are you adding something to the root node? This seems quite strange.
> +name = "cipher"
> +cipher_node = root_node.FindNode(name)
> +if not cipher_node:
> +cipher_node = state.AddSubnode(root_node, name)
> +key_node_name = (
> +
> f"key-{self._algo}-{self._key_name_hint}-{self._iv_name_hint}"
> +if
Re: [PATCH 06/12] binman: Convert mkimage to Entry_section
Hi Jonas,
On Wed, 28 Jun 2023 at 23:35, Jonas Karlman wrote:
>
> Hi Simon,
> On 2023-06-28 13:41, Simon Glass wrote:
> > From: Marek Vasut
> >
> > This is needed to handle mkimage with inner section located itself in a
> > section.
> >
> > Signed-off-by: Marek Vasut
> > Use BuildSectionData() instead of ObtainContents(), add tests and a few
> > other minor fixes:
> > Signed-off-by: Simon Glass
> > ---
> >
> > tools/binman/entry.py | 6 +-
> > tools/binman/etype/mkimage.py | 76 ++-
> > tools/binman/ftest.py | 45 +-
> > tools/binman/test/283_mkimage_special.dts | 24 +++
> > 4 files changed, 117 insertions(+), 34 deletions(-)
> > create mode 100644 tools/binman/test/283_mkimage_special.dts
> >
> > diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> > index 328b5bc568a9..8f06fea51ad4 100644
> > --- a/tools/binman/entry.py
> > +++ b/tools/binman/entry.py
> > @@ -1311,10 +1311,8 @@ features to produce new behaviours.
> > """
> > data = b''
> > for entry in entries:
> > -# First get the input data and put it in a file. If not
> > available,
> > -# try later.
> > -if not entry.ObtainContents(fake_size=fake_size):
> > -return None, None, None
> > +# First get the input data and put it in a file
> > +entry.ObtainContents(fake_size=fake_size)
> > data += entry.GetData()
> > uniq = self.GetUniqueName()
> > fname = tools.get_output_filename(f'{prefix}.{uniq}')
> > diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
> > index cb3e10672ad7..8311fed59762 100644
> > --- a/tools/binman/etype/mkimage.py
> > +++ b/tools/binman/etype/mkimage.py
> > @@ -8,10 +8,11 @@
> > from collections import OrderedDict
> >
> > from binman.entry import Entry
> > +from binman.etype.section import Entry_section
> > from dtoc import fdt_util
> > from u_boot_pylib import tools
> >
> > -class Entry_mkimage(Entry):
> > +class Entry_mkimage(Entry_section):
> > """Binary produced by mkimage
> >
> > Properties / Entry arguments:
> > @@ -121,10 +122,8 @@ class Entry_mkimage(Entry):
> > """
> > def __init__(self, section, etype, node):
> > super().__init__(section, etype, node)
> > -self._mkimage_entries = OrderedDict()
> > self._imagename = None
> > -self._filename = fdt_util.GetString(self._node, 'filename')
> > -self.align_default = None
> > +self._multiple_data_files = False
> >
> > def ReadNode(self):
> > super().ReadNode()
> > @@ -135,41 +134,60 @@ class Entry_mkimage(Entry):
> > 'data-to-imagename')
> > if self._data_to_imagename and self._node.FindNode('imagename'):
> > self.Raise('Cannot use both imagename node and
> > data-to-imagename')
> > -self.ReadEntries()
> >
> > def ReadEntries(self):
> > """Read the subnodes to find out what should go in this image"""
> > for node in self._node.subnodes:
> > -entry = Entry.Create(self, node)
> > +if self.IsSpecialSubnode(node):
> > +continue
> > +entry = Entry.Create(self, node,
> > + expanded=self.GetImage().use_expanded,
> > +
> > missing_etype=self.GetImage().missing_etype)
> > entry.ReadNode()
> > +entry.SetPrefix(self._name_prefix)
> > if entry.name == 'imagename':
> > self._imagename = entry
> > else:
> > -self._mkimage_entries[entry.name] = entry
> > +self._entries[entry.name] = entry
> >
> > -def ObtainContents(self):
> > +def BuildSectionData(self, required):
> > +"""Build mkimage entry contents
> > +
> > +Runs mkimage to build the entry contents
> > +
> > +Args:
> > +required (bool): True if the data must be present, False if it
> > is OK
> > +to return None
> > +
> > +Returns:
> > +bytes: Contents of the section
> > +"""
> > # Use a non-zero size for any fake files to keep mkimage happy
> > # Note that testMkimageImagename() relies on this 'mkimage'
> > parameter
> > fake_size = 1024
> > if self._multiple_data_files:
> > fnames = []
> > uniq = self.GetUniqueName()
> > -for entry in self._mkimage_entries.values():
> > -if not entry.ObtainContents(fake_size=fake_size):
> > -return False
> > -if entry._pathname:
> > -fnames.append(entry._pathname)
> > +for entry in self._entries.values():
> > +entry.ObtainContents(fake_size=fake_size)
> > +
> > +# If
Re: Does CONFIG_I2C_EEPROM depend on CONFIG_MISC_INIT_R?
Hi, On Thu, 29 Jun 2023 at 14:04, wrote: > > Hi, > We noticed that in the code implementation board/atmel/sama7g5ek/sama7g5ek.c, > "#if (IS_ENABLED(CONFIG_I2C_EEPROM))" is located in the condition "#if > (IS_ENABLED(CONFIG_MISC_INIT_R))". > > Consider that the CONFIG_I2C_EEPROM option requires access to EEPROM memory > after normal initial operations are complete. > > Do you think it is necessary to add a dependency on MISC_INIT_R > in the I2C_EEPROM option definition of drivers/misc/Kconfig file? That is a board-specific file, so that board should perhaps always enable include MISC_INIT_R. But other boards may not need this. You could add 'select MISC_INIT_R' to board/atmel/sama7g5ek/Kconfig perhaps, e.g. config BOARD_SPECIFIC_OPTIONS # dummy def_bool y select MISC_INIT_R Regards, Simon
Re: [PATCH 1/2] disk: part: Add API to get partitions with specific driver
Hi Joshua,
On Wed, 28 Jun 2023 at 14:34, Joshua Watt wrote:
>
> On Mon, Jun 26, 2023 at 4:07 AM Simon Glass wrote:
> >
> > Hi Joshua,
> >
> > On Fri, 23 Jun 2023 at 21:01, Joshua Watt wrote:
> > >
> > > Adds part_driver_get_type() API which can be used to force a specific
> > > driver to be used when getting partition information instead of relying
> > > on auto detection.
> > >
> > > Signed-off-by: Joshua Watt
> > > ---
> > > disk/part.c| 38 +++---
> > > include/part.h | 2 ++
> > > 2 files changed, 33 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/disk/part.c b/disk/part.c
> > > index 35300df590..1f8c786ca5 100644
> > > --- a/disk/part.c
> > > +++ b/disk/part.c
> > > @@ -26,6 +26,22 @@
> > > /* Check all partition types */
> > > #define PART_TYPE_ALL -1
> > >
> > > +static struct part_driver *part_driver_get_type(int part_type)
> > > +{
> > > + struct part_driver *drv =
> > > + ll_entry_start(struct part_driver, part_driver);
> > > + const int n_ents = ll_entry_count(struct part_driver,
> > > part_driver);
> > > + struct part_driver *entry;
> > > +
> > > + for (entry = drv; entry != drv + n_ents; entry++) {
> > > + if (part_type == entry->part_type)
> > > + return entry;
> > > + }
> > > +
> > > + /* Not found */
> > > + return NULL;
> > > +}
> > > +
> > > static struct part_driver *part_driver_lookup_type(struct blk_desc
> > > *dev_desc)
> > > {
> > > struct part_driver *drv =
> > > @@ -44,10 +60,7 @@ static struct part_driver
> > > *part_driver_lookup_type(struct blk_desc *dev_desc)
> > > }
> > > }
> > > } else {
> > > - for (entry = drv; entry != drv + n_ents; entry++) {
> > > - if (dev_desc->part_type == entry->part_type)
> > > - return entry;
> > > - }
> > > + return part_driver_get_type(dev_desc->part_type);
> > > }
> > >
> > > /* Not found */
> > > @@ -306,8 +319,8 @@ void part_print(struct blk_desc *dev_desc)
> > > drv->print(dev_desc);
> > > }
> > >
> > > -int part_get_info(struct blk_desc *dev_desc, int part,
> > > - struct disk_partition *info)
> > > +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int
> > > part_type,
> > > + struct disk_partition *info)
> > > {
> > > struct part_driver *drv;
> > >
> > > @@ -320,7 +333,12 @@ int part_get_info(struct blk_desc *dev_desc, int
> > > part,
> > > info->type_guid[0] = 0;
> > > #endif
> > >
> > > - drv = part_driver_lookup_type(dev_desc);
> > > + if (part_type == PART_TYPE_UNKNOWN) {
> > > + drv = part_driver_lookup_type(dev_desc);
> > > + } else {
> > > + drv = part_driver_get_type(part_type);
> > > + }
> > > +
> > > if (!drv) {
> > > debug("## Unknown partition table type %x\n",
> > > dev_desc->part_type);
> > > @@ -340,6 +358,12 @@ int part_get_info(struct blk_desc *dev_desc, int
> > > part,
> > > return -ENOENT;
> > > }
> > >
> > > +int part_get_info(struct blk_desc *dev_desc, int part,
> > > + struct disk_partition *info)
> > > +{
> > > + return part_get_info_by_type(dev_desc, part, PART_TYPE_UNKNOWN,
> > > info);
> > > +}
> > > +
> > > int part_get_info_whole_disk(struct blk_desc *dev_desc,
> > > struct disk_partition *info)
> > > {
> > > diff --git a/include/part.h b/include/part.h
> > > index be75c73549..f150c84206 100644
> > > --- a/include/part.h
> > > +++ b/include/part.h
> > > @@ -106,6 +106,8 @@ struct blk_desc *blk_get_dev(const char *ifname, int
> > > dev);
> > > struct blk_desc *mg_disk_get_dev(int dev);
> > >
> > > /* disk/part.c */
> > > +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int
> > > part_type,
> > > + struct disk_partition *info);
> >
> > Please can you add a full comment?
> >
> > Also please add a test to test/dm/part.c for your new function[1]
>
> Any trick to getting the current test/dm/part.c tests to pass? When I
> run them they fail with `** No device specified **` errors, which
> doesn't give me hope that I can write tests that I could actually run
> locally.
Yes you probably need to run the ''test_ut_dm_init_bootstd' test
first. This pytest is run automatically if you run everything, but
otherwise is not. It sets up the mmc1.img file.
Regards,
Simon
Re: [PATCH 06/10] test: dm: simplify SCMI unit test on sandbox
On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro wrote: > > Adding SCMI base protocol makes it inconvenient to hold the agent instance > (udevice) locally since the agent device will be re-created per each test. > Just remove it and simply the test flows. > The test scenario is not changed at all. > > Signed-off-by: AKASHI Takahiro > --- > arch/sandbox/include/asm/scmi_test.h | 7 ++- > drivers/firmware/scmi/sandbox-scmi_agent.c | 20 +-- > drivers/firmware/scmi/scmi_agent-uclass.c | 14 - > test/dm/scmi.c | 64 +++--- > 4 files changed, 26 insertions(+), 79 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 04/10] sandbox: remove SCMI base node definition from test.dts
On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro wrote: > > SCMI base protocol is mandatory and doesn't need to be listed in a device > tree. > > Signed-off-by: AKASHI Takahiro > --- > arch/sandbox/dts/test.dts | 4 > 1 file changed, 4 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 04/12] binman: blob_dtb: Add fake_size argument to ObtainContents()
On Thu, 29 Jun 2023 at 15:59, wrote: > > From: Lukas Funke > > The method 'connect_contents_to_file()' calls ObtainsContents() with > 'fake_size' argument. Without providing the argument in the blob_dtb > we are not able to call this method without error. > > Signed-off-by: Lukas Funke > --- > > tools/binman/etype/blob_dtb.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH 05/12] binman: doc: Add documentation for fdt_add_pubkey bintool
On Thu, 29 Jun 2023 at 15:59, wrote: > > From: Lukas Funke > > Add documentation for btool which calls 'fdt_add_pubkey' > > Signed-off-by: Lukas Funke > --- > > tools/binman/bintools.rst | 10 ++ > 1 file changed, 10 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH 10/10] test: dm: add scmi command test
On Wed, 28 Jun 2023 at 01:49, AKASHI Takahiro wrote: > > In this test, "scmi" with different sub-commands is tested. > Please note that scmi command is for debug purpose and is not > intended in production system. > > Signed-off-by: AKASHI Takahiro > --- > test/dm/scmi.c | 57 ++ > 1 file changed, 57 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH 02/12] binman: mkimage: Remove extra colon
On 29.06.2023 17:08, Quentin Schulz wrote: Hi Lukas, On 6/29/23 16:59, lukas.funke-...@weidmueller.com wrote: [You don't often get email from lukas.funke-...@weidmueller.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] From: Lukas Funke Remove extra colon typo Signed-off-by: Lukas Funke --- tools/binman/etype/mkimage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index e028c44070..dd734fc779 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -24,7 +24,7 @@ class Entry_mkimage(Entry): - filename: filename of output binary generated by mkimage The data passed to mkimage via the -d flag is collected from subnodes of the - mkimage node, e.g.:: + mkimage node, e.g.: This is by far not the only occurrence in the docstrings, see: $ ag -c :: tools/binman/**/*.py tools/binman/btool/btool_gzip.py:2 tools/binman/btool/bzip2.py:2 tools/binman/btool/lz4.py:2 tools/binman/btool/lzma_alone.py:2 tools/binman/btool/lzop.py:2 tools/binman/btool/xz.py:2 tools/binman/btool/zstd.py:2 tools/binman/cbfs_util.py:1 tools/binman/etype/atf_fip.py:3 tools/binman/etype/cbfs.py:5 tools/binman/etype/fdtmap.py:1 tools/binman/etype/fit.py:7 tools/binman/etype/mkimage.py:7 tools/binman/etype/pre_load.py:1 tools/binman/etype/section.py:1 tools/binman/etype/tee_os.py:2 tools/binman/etype/text.py:3 tools/binman/ftest.py:15 tools/binman/setup.py:3 tools/binman/state.py:1 If I'm not mistaken, we (manually) populate the docstring from the docs written in rST where `::` does actually mean something (start of a literal block, c.f. https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#literal-blocks). I think it's fine to keep it as is (and maybe we could use the docstring directly from within sphinx instead of duplicating it, but I don't have experience with that so I don't know if it's possible, desirable and reasonably easy to implement and maintain). Cheers, Quentin Hi Quentin, I wasn't aware of that, thanks for pointing it out. I'll drop the patch in the next iteration. Best regards Lukas
Re: [PATCH 02/12] binman: mkimage: Remove extra colon
Hi Lukas, On 6/29/23 16:59, lukas.funke-...@weidmueller.com wrote: [You don't often get email from lukas.funke-...@weidmueller.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] From: Lukas Funke Remove extra colon typo Signed-off-by: Lukas Funke --- tools/binman/etype/mkimage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index e028c44070..dd734fc779 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -24,7 +24,7 @@ class Entry_mkimage(Entry): - filename: filename of output binary generated by mkimage The data passed to mkimage via the -d flag is collected from subnodes of the -mkimage node, e.g.:: +mkimage node, e.g.: This is by far not the only occurrence in the docstrings, see: $ ag -c :: tools/binman/**/*.py tools/binman/btool/btool_gzip.py:2 tools/binman/btool/bzip2.py:2 tools/binman/btool/lz4.py:2 tools/binman/btool/lzma_alone.py:2 tools/binman/btool/lzop.py:2 tools/binman/btool/xz.py:2 tools/binman/btool/zstd.py:2 tools/binman/cbfs_util.py:1 tools/binman/etype/atf_fip.py:3 tools/binman/etype/cbfs.py:5 tools/binman/etype/fdtmap.py:1 tools/binman/etype/fit.py:7 tools/binman/etype/mkimage.py:7 tools/binman/etype/pre_load.py:1 tools/binman/etype/section.py:1 tools/binman/etype/tee_os.py:2 tools/binman/etype/text.py:3 tools/binman/ftest.py:15 tools/binman/setup.py:3 tools/binman/state.py:1 If I'm not mistaken, we (manually) populate the docstring from the docs written in rST where `::` does actually mean something (start of a literal block, c.f. https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#literal-blocks). I think it's fine to keep it as is (and maybe we could use the docstring directly from within sphinx instead of duplicating it, but I don't have experience with that so I don't know if it's possible, desirable and reasonably easy to implement and maintain). Cheers, Quentin
[PATCH 07/12] binman: btool: Add fdt_add_pubkey as btool
From: Lukas Funke
Add btool which calls 'fdt_add_pubkey'
Signed-off-by: Lukas Funke
---
tools/binman/btool/fdt_add_pubkey.py | 67
1 file changed, 67 insertions(+)
create mode 100644 tools/binman/btool/fdt_add_pubkey.py
diff --git a/tools/binman/btool/fdt_add_pubkey.py
b/tools/binman/btool/fdt_add_pubkey.py
new file mode 100644
index 00..a50774200c
--- /dev/null
+++ b/tools/binman/btool/fdt_add_pubkey.py
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (C) 2023 Weidmüller Interface GmbH & Co. KG
+# Lukas Funke
+#
+"""Bintool implementation for fdt_add_pubkey"""
+
+from binman import bintool
+
+class Bintoolfdt_add_pubkey(bintool.Bintool):
+"""Add public key to control dtb (spl or u-boot proper)
+
+This bintool supports running `fdt_add_pubkey`.
+
+Normally mkimage adds signature information to the control dtb. However
+binman images are built independent from each other. Thus it is required
+to add the public key separately from mkimage.
+"""
+def __init__(self, name):
+super().__init__(name, 'Generate image for U-Boot')
+
+# pylint: disable=R0913
+def run(self, input_fname, keydir, keyname, required, algo):
+"""Run fdt_add_pubkey
+
+Args:
+input_fname (str): dtb file to sign
+keydir (str): Directory with public key. Optional parameter,
+default value: '.' (current directory)
+keyname (str): Public key name. Optional parameter,
+default value: key
+required (str): If present this indicates that the key must be
+verified for the image / configuration to be considered valid.
+algo (str): Cryptographic algorithm. Optional parameter,
+default value: sha1,rsa2048
+"""
+args = []
+if algo:
+args += ['-a', algo]
+if keydir:
+args += ['-k', keydir]
+if keyname:
+args += ['-n', keyname]
+if required:
+args += ['-r', required]
+
+args += [ input_fname ]
+
+return self.run_cmd(*args)
+
+def fetch(self, method):
+"""Fetch handler for fdt_add_pubkey
+
+This installs fdt_add_pubkey using the apt utility.
+
+Args:
+method (FETCH_...): Method to use
+
+Returns:
+True if the file was fetched and now installed, None if a method
+other than FETCH_BIN was requested
+
+Raises:
+Valuerror: Fetching could not be completed
+"""
+if method != bintool.FETCH_BIN:
+return None
+return self.apt_install('u-boot-tools')
--
2.30.2
[PATCH 08/12] binman: etype: Add u_boot_spl_pubkey_dtb etype
From: Lukas Funke
This adds a new etype 'u_boot_spl_pubkey_dtb'. The etype adds the public
key from a certificate to the dtb. This creates a '/signature' node which
is turn contains the fields which make up the public key. Usually this
is done by 'mkimage -K'. However, 'binman sign' does not add the public
key to the SPL. This is why the pubkey is added using this etype.
The etype calls the underlying 'fdt_add_pubkey' tool.
Signed-off-by: Lukas Funke
---
tools/binman/etype/u_boot_spl_pubkey_dtb.py | 105
1 file changed, 105 insertions(+)
create mode 100644 tools/binman/etype/u_boot_spl_pubkey_dtb.py
diff --git a/tools/binman/etype/u_boot_spl_pubkey_dtb.py
b/tools/binman/etype/u_boot_spl_pubkey_dtb.py
new file mode 100644
index 00..25aa817975
--- /dev/null
+++ b/tools/binman/etype/u_boot_spl_pubkey_dtb.py
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2023 Weidmueller GmbH
+# Written by Lukas Funke
+#
+# Entry-type module for 'u-boot-spl-pubkey.dtb'
+#
+
+import tempfile
+import os
+
+from binman.etype.blob_dtb import Entry_blob_dtb
+
+from dtoc import fdt_util
+
+from u_boot_pylib import tools
+
+# pylint: disable=C0103
+class Entry_u_boot_spl_pubkey_dtb(Entry_blob_dtb):
+"""U-Boot SPL device tree including public key
+
+Properties / Entry arguments:
+- key-name: Public key name without extension (e.g. .crt). Default is
+determined by underlying bintool (fdt_add_pubkey),
+usually 'key'
+- algo: (Optional) Algorithm used for signing. Default is determined by
+underlying bintool (fdt_add_pubkey), usually 'sha1,rsa2048'
+- required: (Optional) If present this indicates that the key must be
+verified for the image / configuration to be
+considered valid
+
+The following example shows an image containing an SPL which
+is packed together with the dtb. Binman will add a signature
+node to the dtb:
+
+image {
+...
+spl {
+filename = "spl.bin"
+
+u_boot_spl_nodtb {
+};
+u_boot_spl_pubkey_dtb {
+algo = "sha384,rsa4096";
+required = "conf";
+key-name = "dev";
+};
+};
+...
+}
+"""
+
+def __init__(self, section, etype, node):
+# Put this here to allow entry-docs and help to work without libfdt
+global state
+from binman import state
+
+super().__init__(section, etype, node)
+self.required_props = ['key-name']
+self.fdt_add_pubkey = None
+self._algo = fdt_util.GetString(self._node, 'algo')
+self._required = fdt_util.GetString(self._node, 'required')
+self._keyname = fdt_util.GetString(self._node, 'key-name')
+
+def ObtainContents(self, fake_size=0):
+""" Add public key which is pointed out by
+'key-name' to node 'signature' in the spl-dtb
+
+This is equivalent to the '-K' option of 'mkimage'
+
+Args:
+fake_size (int): unused
+"""
+
+# We don't pass fake_size and skip_entry upwards
+# because this is currently not support by the blob type
+super().ObtainContents()
+
+with tempfile.NamedTemporaryFile(prefix=os.path.basename(
+ self.GetFdtEtype()),
+ dir=tools.get_output_dir())\
+ as pubkey_tdb:
+tools.write_file(pubkey_tdb.name, self.GetData())
+keyname = tools.get_input_filename(self._keyname + ".crt")
+self.fdt_add_pubkey.run(pubkey_tdb.name,
+os.path.dirname(keyname),
+self._keyname,
+self._required, self._algo)
+dtb = tools.read_file(pubkey_tdb.name)
+self.SetContents(dtb)
+state.UpdateFdtContents(self.GetFdtEtype(), dtb)
+
+return True
+
+# pylint: disable=R0201,C0116
+def GetDefaultFilename(self):
+return 'spl/u-boot-spl-pubkey.dtb'
+
+# pylint: disable=R0201,C0116
+def GetFdtEtype(self):
+return 'u-boot-spl-dtb'
+
+# pylint: disable=R0201,C0116
+def AddBintools(self, btools):
+super().AddBintools(btools)
+self.fdt_add_pubkey = self.AddBintool(btools, 'fdt_add_pubkey')
--
2.30.2
[PATCH 03/12] binman: Don't decompress data while signing
From: Lukas Funke While signing a fit compressed data (i.e. 'blob-ext') is decompressed, but never compressed again. When compressed data was wrapped in a section, decompression leads to an error because the outer section had the original compressed size but the inner entry has the uncompressed size now. While singing there is no reason to decompress data. Thus, decompression should be disabled. Furthermore, bintools should be collected before loading the data. This way bintools are available if processing is required on a node. Signed-off-by: Lukas Funke --- tools/binman/control.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/binman/control.py b/tools/binman/control.py index 68597c4e77..affc33ff3d 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -306,8 +306,8 @@ def BeforeReplace(image, allow_resize): image: Image to prepare """ state.PrepareFromLoadedData(image) -image.LoadData() image.CollectBintools() +image.LoadData(decomp=False) # If repacking, drop the old offset/size values except for the original # ones, so we are only left with the constraints. -- 2.30.2
[PATCH 02/12] binman: mkimage: Remove extra colon
From: Lukas Funke
Remove extra colon typo
Signed-off-by: Lukas Funke
---
tools/binman/etype/mkimage.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index e028c44070..dd734fc779 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -24,7 +24,7 @@ class Entry_mkimage(Entry):
- filename: filename of output binary generated by mkimage
The data passed to mkimage via the -d flag is collected from subnodes of
the
-mkimage node, e.g.::
+mkimage node, e.g.:
mkimage {
filename = "imximage.bin";
--
2.30.2
[PATCH 09/12] binman: doc: Add documentation for Xilinx Bootgen bintool
From: Lukas Funke Add documentation for the 'bootgen' bintool Signed-off-by: Lukas Funke --- tools/binman/bintools.rst | 12 1 file changed, 12 insertions(+) diff --git a/tools/binman/bintools.rst b/tools/binman/bintools.rst index 88221adbe1..c8d69f7177 100644 --- a/tools/binman/bintools.rst +++ b/tools/binman/bintools.rst @@ -193,3 +193,15 @@ Normally signing is done using `mkimage` in context of `binman sign`. However, in this process the public key is not added to the stage before u-boot proper. Using `fdt_add_pubkey` the key can be injected to the SPL independent of `mkimage` + + + +Bintool: bootgen: Sign ZynqMP FSBL image +- + +This bintool supports running `bootgen` in order to sign a SPL for ZynqMP +devices. + +The bintool automatically creates an appropriate input image file (.bif) for +bootgen based on the passed arguments. The output is a bootable, +authenticated `boot.bin` file. -- 2.30.2
[PATCH 11/12] binman: ftest: Add test for xilinx_fsbl_auth etype
From: Lukas Funke
Add test for the 'xilinx_fsbl_auth' etype
Signed-off-by: Lukas Funke
---
tools/binman/ftest.py | 8
tools/binman/test/280_xilinx_fsb_auth.dts | 22 ++
2 files changed, 30 insertions(+)
create mode 100644 tools/binman/test/280_xilinx_fsb_auth.dts
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 3bd09d3fea..f0a7861649 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6686,6 +6686,14 @@ fdt fdtmapExtract the devicetree
blob from the fdtmap
['fit'])
self.assertIn("Node '/fit': Missing tool: 'mkimage'", str(e.exception))
+def testXilinxFsblAuth(self):
+"""Test xilinx_fsbl_auth etype"""
+data = tools.read_file(self.TestFile("key.key"))
+self._MakeInputFile("psk.pem", data)
+self._MakeInputFile("ssk.pem", data)
+self._SetupPmuFwlElf()
+self._SetupSplElf()
+self._DoReadFileRealDtb('280_xilinx_fsb_auth.dts')
def testSplPubkeyDtb(self):
"""Test u_boot_spl_pubkey_dtb etype"""
diff --git a/tools/binman/test/280_xilinx_fsb_auth.dts
b/tools/binman/test/280_xilinx_fsb_auth.dts
new file mode 100644
index 00..2bfd36c22e
--- /dev/null
+++ b/tools/binman/test/280_xilinx_fsb_auth.dts
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ xilinx_fsbl_auth {
+
+ psk-filename = "psk.pem";
+ ssk-filename = "ssk.pem";
+ auth-params = "ppk_select=0", "spk_id=0x";
+
+ u_boot_spl_nodtb {
+ };
+ u_boot_spl_dtb {
+ };
+ };
+ };
+};
--
2.30.2
[PATCH 05/12] binman: doc: Add documentation for fdt_add_pubkey bintool
From: Lukas Funke Add documentation for btool which calls 'fdt_add_pubkey' Signed-off-by: Lukas Funke --- tools/binman/bintools.rst | 10 ++ 1 file changed, 10 insertions(+) diff --git a/tools/binman/bintools.rst b/tools/binman/bintools.rst index c30e7eb9ff..88221adbe1 100644 --- a/tools/binman/bintools.rst +++ b/tools/binman/bintools.rst @@ -183,3 +183,13 @@ Documentation is available via:: +Bintool: fdt_add_pubkey: Add public key to device tree +- + +This bintool supports running `fdt_add_pubkey` in order to add a public +key coming from a certificate to a device-tree. + +Normally signing is done using `mkimage` in context of `binman sign`. However, +in this process the public key is not added to the stage before u-boot proper. +Using `fdt_add_pubkey` the key can be injected to the SPL independent of +`mkimage` -- 2.30.2
[PATCH 06/12] binman: ftest: Add test for u_boot_spl_pubkey_dtb
From: Lukas Funke
Add test for u_boot_spl_pubkey_dtb. The test adds a public key to the
dtb and checks if the required nodes will be added to the images dtb.
Signed-off-by: Lukas Funke
---
tools/binman/ftest.py| 32
tools/binman/test/281_spl_pubkey_dtb.dts | 16
2 files changed, 48 insertions(+)
create mode 100644 tools/binman/test/281_spl_pubkey_dtb.dts
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 43b4f850a6..3bd09d3fea 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -638,6 +638,16 @@ class TestFunctional(unittest.TestCase):
TestFunctional._MakeInputFile('vpl/u-boot-vpl',
tools.read_file(cls.ElfTestFile(src_fname)))
+@classmethod
+def _SetupPmuFwlElf(cls, src_fname='bss_data'):
+"""Set up an ELF file with a '_dt_ucode_base_size' symbol
+
+Args:
+Filename of ELF file to use as VPL
+"""
+TestFunctional._MakeInputFile('pmu-firmware.elf',
+tools.read_file(cls.ElfTestFile(src_fname)))
+
@classmethod
def _SetupDescriptor(cls):
with open(cls.TestFile('descriptor.bin'), 'rb') as fd:
@@ -6677,5 +6687,27 @@ fdt fdtmapExtract the devicetree
blob from the fdtmap
self.assertIn("Node '/fit': Missing tool: 'mkimage'", str(e.exception))
+def testSplPubkeyDtb(self):
+ """Test u_boot_spl_pubkey_dtb etype"""
+ data = tools.read_file(self.TestFile("key.pem"))
+ self._MakeInputFile("key.crt", data)
+ self._DoReadFileRealDtb('281_spl_pubkey_dtb.dts')
+ image = control.images['image']
+ entries = image.GetEntries()
+ dtb_entry = entries['u_boot_spl_pubkey_dtb']
+ dtb_data = dtb_entry.GetData()
+ dtb = fdt.Fdt.FromData(dtb_data)
+ dtb.Scan()
+
+ signature_node = dtb.GetNode('/signature')
+ self.assertIsNotNone(signature_node)
+ key_node = signature_node.FindNode("key-key")
+ self.assertIsNotNone(key_node)
+ self.assertEqual(fdt_util.GetString(key_node, "required"),
+ "conf")
+ self.assertEqual(fdt_util.GetString(key_node, "algo"),
+ "sha384,rsa4096")
+ self.assertEqual(fdt_util.GetString(key_node, "key-name-hint"),
+ "key")
if __name__ == "__main__":
unittest.main()
diff --git a/tools/binman/test/281_spl_pubkey_dtb.dts
b/tools/binman/test/281_spl_pubkey_dtb.dts
new file mode 100644
index 00..5a2952ed7d
--- /dev/null
+++ b/tools/binman/test/281_spl_pubkey_dtb.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ u_boot_spl_pubkey_dtb {
+ algo = "sha384,rsa4096";
+ required = "conf";
+ key-name = "key";
+ };
+ };
+};
--
2.30.2
[PATCH 12/12] binman: etype: Add xilinx_fsbl_auth etype
From: Lukas Funke
This adds a new etype 'xilinx_fsbl_auth'. Using this etype it is possible
to created an authenticated SPL (FSBL in Xilinx terms) for ZynqMP boards.
The etype uses Xilinx Bootgen tools in order to transform the SPL into
a bootable image and sign the image with a given primary and seconrady
public key. For more information to signing the FSBL please refer to the
Xilinx Bootgen documentation.
Here is an example of the etype in use:
spl {
filename = "boot.signed.bin";
xilinx_fsbl_auth {
psk-filename = "psk0.pem";
ssk-filename = "ssk0.pem";
auth-params = "ppk_select=0", "spk_id=0x";
u_boot_spl_nodtb {
};
u_boot_spl_dtb {
};
};
};
For this to work the hash of the primary public key has to be fused
into the ZynqMP device and authentication (RSA_EN) has to be set.
For testing purposes: if ppk hash check should be skipped one can add
the property 'fsbl_config = "bh_auth_enable";' to the etype. However,
this should only be used for testing(!).
Signed-off-by: Lukas Funke
---
tools/binman/etype/xilinx_fsbl_auth.py | 186 +
1 file changed, 186 insertions(+)
create mode 100644 tools/binman/etype/xilinx_fsbl_auth.py
diff --git a/tools/binman/etype/xilinx_fsbl_auth.py
b/tools/binman/etype/xilinx_fsbl_auth.py
new file mode 100644
index 00..ec70db9414
--- /dev/null
+++ b/tools/binman/etype/xilinx_fsbl_auth.py
@@ -0,0 +1,186 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2023 Weidmueller GmbH
+# Written by Lukas Funke
+#
+# Entry-type module for signed ZynqMP boot images (boot.bin)
+#
+
+import tempfile
+
+from collections import OrderedDict
+
+from binman import elf
+from binman.entry import Entry
+
+from dtoc import fdt_util
+
+from u_boot_pylib import tools
+from u_boot_pylib import command
+
+# pylint: disable=C0103
+class Entry_xilinx_fsbl_auth(Entry):
+"""Authenticated SPL flat binary for booting Xilinx
+ZynqMP devices
+
+Properties / Entry arguments:
+- auth-params: (Optional) Authentication parameters passed to bootgen
+- fsbl-config: (Optional) FSBL parameters passed to bootgen
+- pmufw-filename: Filename of PMU firmware. Default: pmu-firmware.elf
+- psk-filename: Filename of primary public key
+- ssk-filename: Filename of secondary public key
+
+The following example builds an authenticated boot image. The fuses of
+the primary public key (ppk) should be fused together with the RSA_EN flag.
+
+spl {
+filename = "boot.signed.bin";
+
+xilinx_fsbl_auth {
+psk-filename = "psk0.pem";
+ssk-filename = "ssk0.pem";
+auth-params = "ppk_select=0", "spk_id=0x";
+
+u_boot_spl_nodtb {
+};
+u_boot_spl_pubkey_dtb {
+algo = "sha384,rsa4096";
+required = "conf";
+key-name = "dev";
+};
+};
+};
+
+For testing purposes, e.g. if no RSA_EN should be fused, one could add
+the "bh_auth_enable" flag in the fsbl-config field. This will skip the
+verification of the ppk fuses and boot the image, even if ppk hash is
+invalid:
+
+xilinx_fsbl_auth {
+psk-filename = "psk0.pem";
+ssk-filename = "ssk0.pem";
+...
+fsbl-config = "bh_auth_enable";
+...
+};
+
+"""
+def __init__(self, section, etype, node):
+super().__init__(section, etype, node)
+self.align_default = None
+self.bootgen = None
+self._entries = OrderedDict()
+self._filename = self.GetDefaultFilename()
+self.required_props = ['psk-filename', 'ssk-filename']
+
+def ReadNode(self):
+"""Read properties from the xilinx_fsbl_auth node"""
+super().ReadNode()
+self._auth_params = fdt_util.GetStringList(self._node,
+ 'auth-params')
+self._fsbl_config = fdt_util.GetStringList(self._node,
+ 'fsbl-config')
+self._pmufw_filename = fdt_util.GetString(self._node,
+ 'pmufw-filename',
+ 'pmu-firmware.elf')
+self._psk_filename = fdt_util.GetString(self._node, 'psk-filename',
+'psk.pem')
+self._ssk_filename = fdt_util.GetString(self._node, 'ssk-filename',
+'ssk.pem')
+self.ReadEntries()
+
+def ReadEntries(self):
+"""Read the subnodes to find out what should go in this image"""
+for node in self._node.subnodes:
+entry = Entry.Create(self, node)
+entry.ReadNode()
+self._entries[entry.name] = entry
+
+@classmethod
+def __ToElf(self, data,
[PATCH 10/12] binman: btool: Add Xilinx Bootgen btool
From: Lukas Funke
Add the Xilinx Bootgen as bintool. Xilinx Bootgen is used to create
bootable SPL (FSBL in Xilinx terms) images for Zynq/ZynqMP devices. The
btool creates a signed version of the SPL.
Signed-off-by: Lukas Funke
---
tools/binman/btool/bootgen.py | 82 +++
1 file changed, 82 insertions(+)
create mode 100644 tools/binman/btool/bootgen.py
diff --git a/tools/binman/btool/bootgen.py b/tools/binman/btool/bootgen.py
new file mode 100644
index 00..8bc727a54f
--- /dev/null
+++ b/tools/binman/btool/bootgen.py
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (C) 2023 Weidmüller Interface GmbH & Co. KG
+# Lukas Funke
+#
+"""Bintool implementation for bootgen
+
+bootgen allows creating bootable SPL for Zynq(MP)
+
+Documentation is available via::
+https://www.xilinx.com/support/documents/sw_manuals/xilinx2022_1/ug1283-bootgen-user-guide.pdf
+
+Source code is available at:
+
+https://github.com/Xilinx/bootgen
+
+"""
+import tempfile
+
+from binman import bintool
+from u_boot_pylib import tools
+
+# pylint: disable=C0103
+class Bintoolbootgen(bintool.Bintool):
+"""Generate bootable fsbl image for zynq/zynqmp
+
+This bintools supports running Xilinx "bootgen" in order
+to generate a bootable, authenticated image form an SPL.
+
+"""
+def __init__(self, name):
+super().__init__(name, 'Xilinx Bootgen',
+ version_regex=r'^\*\*\*\*\*\* Xilinx Bootgen',
+ version_args='')
+
+# pylint: disable=R0913
+def sign(self, arch, spl_elf_fname, pmufw_elf_fname,
+ psk_fname, ssk_fname, fsbl_config, auth_params, output_fname):
+""" Sign FSBL(SPL) elf file and bundle it with pmu firmware
+to a bootable image
+
+Args:
+arch (str): Xilinx SoC architecture
+spl_elf_fname (str): Filename of FSBL ELF file
+pmufw_elf_fname (str): Filename pmu firmware
+psk_fname (str): Filename of the primary secret key
+ssk_fname (str): Filename of the secondary secret key
+fsbl_config (str): FSBL config options
+auth_params (str): Authentication parameter
+output_fname (str): Filename where bootgen should write the result
+"""
+
+_fsbl_config = f"[fsbl_config] {fsbl_config}" if fsbl_config else ""
+_auth_params = f"[auth_params] {auth_params}" if auth_params else ""
+
+bif_template = f"""u_boot_spl_aes_rsa: {{
+[pskfile] {psk_fname}
+[sskfile] {ssk_fname}
+{_fsbl_config}
+{_auth_params}
+[ bootloader,
+ authentication = rsa,
+ destination_cpu=a53-0] {spl_elf_fname}
+[pmufw_image] {pmufw_elf_fname}
+}}"""
+args = ["-arch", arch]
+
+with tempfile.NamedTemporaryFile(suffix=".bif",
+ dir=tools.get_output_dir()) as bif:
+tools.write_file(bif.name, bif_template, binary=False)
+args += ["-image", bif.name, '-w', '-o', output_fname]
+self.run_cmd(*args)
+
+def fetch(self, method):
+"""Fetch bootgen from git"""
+if method != bintool.FETCH_BUILD:
+return None
+
+result = self.build_from_git(
+'https://github.com/Xilinx/bootgen',
+'all',
+'build/bootgen/bootgen')
+return result
--
2.30.2
[PATCH 04/12] binman: blob_dtb: Add fake_size argument to ObtainContents()
From: Lukas Funke
The method 'connect_contents_to_file()' calls ObtainsContents() with
'fake_size' argument. Without providing the argument in the blob_dtb
we are not able to call this method without error.
Signed-off-by: Lukas Funke
---
tools/binman/etype/blob_dtb.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index 6a3fbc4775..d543de9f75 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -38,7 +38,7 @@ class Entry_blob_dtb(Entry_blob):
self.Raise("Invalid prepend in '%s': '%s'" %
(self._node.name, self.prepend))
-def ObtainContents(self):
+def ObtainContents(self, fake_size=0):
"""Get the device-tree from the list held by the 'state' module"""
self._filename = self.GetDefaultFilename()
self._pathname, _ = state.GetFdtContents(self.GetFdtEtype())
--
2.30.2
[PATCH 00/12] Sign Xilinx ZynqMP SPL/FSBL boot images using binman
From: Lukas Funke
This series adds two etypes to create a verified boot chain for
Xilinx ZynqMP devices. The first etype 'xilinx_fsbl_auth' is used to
create a bootable, signed image for ZynqMP boards using the Xilinx
Bootgen tool. The second etype 'u_boot_spl_pubkey_dtb' is used to add
a '/signature' node to the SPL. The public key in the signature is read
from a certificate file and added using the 'fdt_add_pubkey' tool. The
series also contains the corresponding btool for calling 'bootgen' and
'fdt_add_pubkey'
The following block shows an example on how to use this functionality:
spl {
filename = "boot.signed.bin";
xilinx_fsbl_auth {
psk-filename = "psk0.pem";
ssk-filename = "ssk0.pem";
auth-params = "ppk_select=0", "spk_id=0x";
u_boot_spl_nodtb {
};
u_boot_spl_pubkey_dtb {
algo = "sha384,rsa4096";
required = "conf";
key-name = "dev";
};
};
};
Lukas Funke (12):
binman: elf: Check for ELF_TOOLS availability and remove extra
semicolon
binman: mkimage: Remove extra colon
binman: Don't decompress data while signing
binman: blob_dtb: Add fake_size argument to ObtainContents()
binman: doc: Add documentation for fdt_add_pubkey bintool
binman: ftest: Add test for u_boot_spl_pubkey_dtb
binman: btool: Add fdt_add_pubkey as btool
binman: etype: Add u_boot_spl_pubkey_dtb etype
binman: doc: Add documentation for Xilinx Bootgen bintool
binman: btool: Add Xilinx Bootgen btool
binman: ftest: Add test for xilinx_fsbl_auth etype
binman: etype: Add xilinx_fsbl_auth etype
tools/binman/bintools.rst | 22 +++
tools/binman/btool/bootgen.py | 82 +
tools/binman/btool/fdt_add_pubkey.py| 67 +++
tools/binman/control.py | 2 +-
tools/binman/elf.py | 10 +-
tools/binman/etype/blob_dtb.py | 2 +-
tools/binman/etype/mkimage.py | 2 +-
tools/binman/etype/u_boot_spl_pubkey_dtb.py | 105 +++
tools/binman/etype/xilinx_fsbl_auth.py | 186
tools/binman/ftest.py | 42 -
tools/binman/test/280_xilinx_fsb_auth.dts | 22 +++
tools/binman/test/281_spl_pubkey_dtb.dts| 16 ++
12 files changed, 550 insertions(+), 8 deletions(-)
create mode 100644 tools/binman/btool/bootgen.py
create mode 100644 tools/binman/btool/fdt_add_pubkey.py
create mode 100644 tools/binman/etype/u_boot_spl_pubkey_dtb.py
create mode 100644 tools/binman/etype/xilinx_fsbl_auth.py
create mode 100644 tools/binman/test/280_xilinx_fsb_auth.dts
create mode 100644 tools/binman/test/281_spl_pubkey_dtb.dts
--
2.30.2
[PATCH 01/12] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon
From: Lukas Funke
Check if elf tools are available when running DecodeElf(). Also
remove superfuous semicolon at line ending.
Signed-off-by: Lukas Funke
---
tools/binman/elf.py | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/binman/elf.py b/tools/binman/elf.py
index 5816284c32..a53f4b9c4f 100644
--- a/tools/binman/elf.py
+++ b/tools/binman/elf.py
@@ -438,13 +438,15 @@ def DecodeElf(data, location):
Returns:
ElfInfo object containing information about the decoded ELF file
"""
+if not ELF_TOOLS:
+raise ValueError("Python: No module named 'elftools'")
file_size = len(data)
with io.BytesIO(data) as fd:
elf = ELFFile(fd)
-data_start = 0x;
-data_end = 0;
-mem_end = 0;
-virt_to_phys = 0;
+data_start = 0x
+data_end = 0
+mem_end = 0
+virt_to_phys = 0
for i in range(elf.num_segments()):
segment = elf.get_segment(i)
--
2.30.2
v2023.07-rc5 regression: Image overlaps SPL
Hello Marek,
as briefly discussed off-list it looks like
commit 77aed22b48ab ("spl: spl_legacy: Add extra address checks") introduces a
regression on some board/arch, at least colibri and apalis imx6 fails to boot
now
```
Trying to boot from MMC1
SPL: Image overlaps SPL
resetting ...
```
>From a quick check verdin imx8mm and imx8mp are not affected.
I also noticed something weird on a colibri imx7s, this is not using SPL,
likely something completly different, however given this is new also from rc5 I
thought it's valuable to report:
```
U-Boot 2023.07-rc5-0.0.0-devel+git.580eb31199be (Jun 27 2023 - 13:39:58 +)
CPU: Freescale i.MX7S rev1.2 800 MHz (running at 792 MHz)
CPU: Extended Commercial temperature grade (-20C to 105C) at 30C
Reset cause: POR
DRAM: initcall sequence 8786eafc failed at call 8781b351 (err=-3)
### ERROR ### Please RESET the board ###
```
Currently at the EOSS in Prague, so no way to do more testing/debugging, but I
tought it was better to start reporting this on the list since we are getting
close to the actual release.
Francesco
Re: [PATCH] rockchip: Restore support for boot scripts in legacy image format
Hi Kever,
On 2023-06-29 12:51, Kever Yang wrote:
> Hi Jonas,
>
> What is the legacy image format, does these boards really use than?
>
> I think most of the board using linux distro image.
The legacy image format it the normal boot script format,
in the documentation it is called Legacy U-Boot image, see [1].
i.e. boot scripts compiled with:
mkimage -T script -n 'Test script' -d boot.txt boot.scr
Such boot scripts are used by some linux distro, e.g. Armbian.
[1]
https://u-boot.readthedocs.io/en/latest/usage/cmd/source.html#legacy-u-boot-image
Regards,
Jonas
>
>
> Thanks,
>
> - Kever
>
> On 2023/6/27 03:43, Jonas Karlman wrote:
>> Use of CONFIG_SPL_FIT_SIGNATURE=y cause CONFIG_LEGACY_IMAGE_FORMAT=n as
>> default, this prevent boot scripts in legacy image format from working
>> and was an unintended change in the listed fixes commits:
>>
>>Wrong image format for "source" command
>>
>> Add CONFIG_LEGACY_IMAGE_FORMAT=y to defconfig for affected boards to
>> restore support for boot scripts in legacy image format.
>>
>> Fixes: 3bf8e4080763 ("board: rockchip: add Radxa ROCK5B Rk3588 board")
>> Fixes: cf777572ca31 ("rockchip: rockpro64: Use SDMA to boost eMMC
>> performance")
>> Fixes: 6e2b8344d60c ("rockchip: rock-pi-4: Use SDMA to boost eMMC
>> performance")
>> Fixes: 1bf49d5a4a7c ("rockchip: rk3566-radxa-cm3-io: Update defconfig")
>> Fixes: 703c170b40f2 ("rockchip: rk3568-evb: Update defconfig")
>> Fixes: 68000f750acd ("rockchip: rk3568-rock-3a: Update defconfig")
>> Fixes: 6fb02589a608 ("rockchip: rk3588-evb: Update defconfig")
>> Signed-off-by: Jonas Karlman
>> ---
>> configs/evb-rk3568_defconfig | 1 +
>> configs/evb-rk3588_defconfig | 1 +
>> configs/radxa-cm3-io-rk3566_defconfig | 1 +
>> configs/rock-3a-rk3568_defconfig | 1 +
>> configs/rock-pi-4-rk3399_defconfig| 1 +
>> configs/rock5b-rk3588_defconfig | 1 +
>> configs/rockpro64-rk3399_defconfig| 1 +
>> 7 files changed, 7 insertions(+)
>>
>> diff --git a/configs/evb-rk3568_defconfig b/configs/evb-rk3568_defconfig
>> index 07819d105441..5f3fab7304c2 100644
>> --- a/configs/evb-rk3568_defconfig
>> +++ b/configs/evb-rk3568_defconfig
>> @@ -22,6 +22,7 @@ CONFIG_FIT=y
>> CONFIG_FIT_VERBOSE=y
>> CONFIG_SPL_FIT_SIGNATURE=y
>> CONFIG_SPL_LOAD_FIT=y
>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>> CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-evb.dtb"
>> # CONFIG_DISPLAY_CPUINFO is not set
>> CONFIG_DISPLAY_BOARDINFO_LATE=y
>> diff --git a/configs/evb-rk3588_defconfig b/configs/evb-rk3588_defconfig
>> index d5f1c4b9ebc7..f49c2ca686a8 100644
>> --- a/configs/evb-rk3588_defconfig
>> +++ b/configs/evb-rk3588_defconfig
>> @@ -23,6 +23,7 @@ CONFIG_FIT=y
>> CONFIG_FIT_VERBOSE=y
>> CONFIG_SPL_FIT_SIGNATURE=y
>> CONFIG_SPL_LOAD_FIT=y
>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>> CONFIG_OF_BOARD_SETUP=y
>> CONFIG_DEFAULT_FDT_FILE="rockchip/rk3588-evb1-v10.dtb"
>> # CONFIG_DISPLAY_CPUINFO is not set
>> diff --git a/configs/radxa-cm3-io-rk3566_defconfig
>> b/configs/radxa-cm3-io-rk3566_defconfig
>> index 56802d85cc25..488723dfaa30 100644
>> --- a/configs/radxa-cm3-io-rk3566_defconfig
>> +++ b/configs/radxa-cm3-io-rk3566_defconfig
>> @@ -22,6 +22,7 @@ CONFIG_FIT=y
>> CONFIG_FIT_VERBOSE=y
>> CONFIG_SPL_FIT_SIGNATURE=y
>> CONFIG_SPL_LOAD_FIT=y
>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>> CONFIG_DEFAULT_FDT_FILE="rockchip/rk3566-radxa-cm3-io.dtb"
>> # CONFIG_DISPLAY_CPUINFO is not set
>> CONFIG_DISPLAY_BOARDINFO_LATE=y
>> diff --git a/configs/rock-3a-rk3568_defconfig
>> b/configs/rock-3a-rk3568_defconfig
>> index 616499f2f82b..753d03914d90 100644
>> --- a/configs/rock-3a-rk3568_defconfig
>> +++ b/configs/rock-3a-rk3568_defconfig
>> @@ -27,6 +27,7 @@ CONFIG_FIT=y
>> CONFIG_FIT_VERBOSE=y
>> CONFIG_SPL_FIT_SIGNATURE=y
>> CONFIG_SPL_LOAD_FIT=y
>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>> CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-rock-3a.dtb"
>> # CONFIG_DISPLAY_CPUINFO is not set
>> CONFIG_DISPLAY_BOARDINFO_LATE=y
>> diff --git a/configs/rock-pi-4-rk3399_defconfig
>> b/configs/rock-pi-4-rk3399_defconfig
>> index 4b984adc6ef8..466868d80b03 100644
>> --- a/configs/rock-pi-4-rk3399_defconfig
>> +++ b/configs/rock-pi-4-rk3399_defconfig
>> @@ -20,6 +20,7 @@ CONFIG_PCI=y
>> CONFIG_DEBUG_UART=y
>> # CONFIG_ANDROID_BOOT_IMAGE is not set
>> CONFIG_SPL_FIT_SIGNATURE=y
>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>> CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-rock-pi-4a.dtb"
>> CONFIG_DISPLAY_BOARDINFO_LATE=y
>> CONFIG_MISC_INIT_R=y
>> diff --git a/configs/rock5b-rk3588_defconfig
>> b/configs/rock5b-rk3588_defconfig
>> index c1155c20efa8..17205a56cd99 100644
>> --- a/configs/rock5b-rk3588_defconfig
>> +++ b/configs/rock5b-rk3588_defconfig
>> @@ -29,6 +29,7 @@ CONFIG_FIT=y
>> CONFIG_FIT_VERBOSE=y
>> CONFIG_SPL_FIT_SIGNATURE=y
>> CONFIG_SPL_LOAD_FIT=y
>> +CONFIG_LEGACY_IMAGE_FORMAT=y
>> CONFIG_OF_BOARD_SETUP=y
>> CONFIG_DEFAULT_FDT_FILE="rockchip/rk3588-rock-5b.dtb"
>> # CONFIG_DISPLAY_CPUINFO is not
[PATCH v3] arm: dts: am335x-evm-u-boot: Remove usb0 mode configuration
This USB port configuration is moved to 'am335x-evm.dts'.
Signed-off-by: Julien Panis
---
Initially, this series was submitted to fix usb0 dr_mode
for am335x-icev2 and am335x-evmsk. It must be set to
'peripheral' in order to avoid 'no USB device found'
error, in usb_ether_init() function.
It was finally decided to move usb0 mode configuration
in kernel device trees. That's why a series was submitted
for linux:
https://lore.kernel.org/all/20230629-usb0-as-peripheral-v1-0-167f78a11...@baylibre.com/
As a consequence, usb0 dr_mode configuration must now be
removed from 'am335x-evm-u-boot.dtsi', since this fragment
needs to go in linux too.
---
Changes in v3:
- Remove usb0 dr_mode configuration from 'am335x-evm-u-boot.dtsi'.
- Link to v2:
https://lore.kernel.org/r/20230621-fix_usb_ether_init-v2-0-ff121f0e8...@baylibre.com
Changes in v2:
- Drop the modification made in arch/arm/mach-omap2/am33xx/board.c
- Configure usb0 dr_mode as peripheral in 'am335x-icev2-u-boot.dtsi'
and 'am335x-evmsk-u-boot.dtsi' device trees.
- Link to v1:
https://lore.kernel.org/r/20230621-fix_usb_ether_init-v1-1-215692399...@baylibre.com
---
arch/arm/dts/am335x-evm-u-boot.dtsi | 4
1 file changed, 4 deletions(-)
diff --git a/arch/arm/dts/am335x-evm-u-boot.dtsi
b/arch/arm/dts/am335x-evm-u-boot.dtsi
index 82a483ae3e28..42eeca8c0499 100644
--- a/arch/arm/dts/am335x-evm-u-boot.dtsi
+++ b/arch/arm/dts/am335x-evm-u-boot.dtsi
@@ -23,10 +23,6 @@
status = "disabled";
};
- {
- dr_mode = "peripheral";
-};
-
{
bootph-all;
};
---
base-commit: 19b77d3d23966a0d6dbb3c86187765f11100fb6f
change-id: 20230621-fix_usb_ether_init-4bf4f1135113
Best regards,
--
Julien Panis
Does CONFIG_I2C_EEPROM depend on CONFIG_MISC_INIT_R?
Hi, We noticed that in the code implementation board/atmel/sama7g5ek/sama7g5ek.c, "#if (IS_ENABLED(CONFIG_I2C_EEPROM))" is located in the condition "#if (IS_ENABLED(CONFIG_MISC_INIT_R))". Consider that the CONFIG_I2C_EEPROM option requires access to EEPROM memory after normal initial operations are complete. Do you think it is necessary to add a dependency on MISC_INIT_R in the I2C_EEPROM option definition of drivers/misc/Kconfig file? Best regards, Yanjie Ren Ying Sun
Suggestion: Does the CONFIG_RTC_DS1307 and CONFIG_DM_RTC options conflict?
hi, we recently noticed that RTC_DS1307 is dependent on DM_RTC in drivers/rtc/Kconfig, but the code implementation of drivers/rtc/ds1307.c: conflicts between the two. "#ifdef CONFIG_RTC_DS1307" is in the condition "#ifndef CONFIG_DM_RTC" . These two options are used to enable different RTC drivers. If both options are enabled, there is a situation where two RTC drivers exist at the same time, which can lead to conflicts and errors. Therefore, only one of these two options should be enabled, not both. Do you think it is necessary to fix the dependency in drivers/rtc/Kconfig? Best regards, Yanjie Ren Ying Sun
Re: [PATCH v2 0/2] Fix 'no USB device found' error.
On 6/26/23 09:32, Nishanth Menon wrote: On 13:59-20230623, Tom Rini wrote: On Fri, Jun 23, 2023 at 09:42:01AM +0200, Julien Panis wrote: On 6/22/23 17:49, Tom Rini wrote: On Thu, Jun 22, 2023 at 04:34:34PM +0200, Julien Panis wrote: This series fixes usb0 dr_mode for am335x-icev2 and am335x-evmsk. It must be set to 'peripheral' in order to avoid 'no USB device found' error, in usb_ether_init() function. Signed-off-by: Julien Panis --- Changes in v2: - Drop the modification made in arch/arm/mach-omap2/am33xx/board.c - Configure usb0 dr_mode as peripheral in 'am335x-icev2-u-boot.dtsi' and 'am335x-evmsk-u-boot.dtsi' device trees. - Link to v1: https://lore.kernel.org/r/20230621-fix_usb_ether_init-v1-1-215692399...@baylibre.com --- Julien Panis (2): arm: dts: am335x-icev2-u-boot: Configure peripheral mode for usb0 arm: dts: am335x-evmsk-u-boot: Configure peripheral mode for usb0 arch/arm/dts/am335x-evmsk-u-boot.dtsi | 4 arch/arm/dts/am335x-icev2-u-boot.dtsi | 4 2 files changed, 8 insertions(+) I'll ask the first question that Nishanth might also ask, which is why don't these belong in the kernel dts files? Thanks! That's a good question. :) Looping Nishanth... usb0 dr_mode property is already overlayed for am335x-evm, in 'am335x-evm-u-boot.dtsi'. So, it appeared more consistent to me to do the same thing for am335x-icev2 and am335x-evmsk. I guess that the goal is to configure usb0 as host by default at kernel boot, since we do not necessarily want to use this usb0 as peripheral from userspace. Is it the right explanation @Vignesh @Nishanth ? It's I think even more likely that the am335x_evm fragment needs to go upstream too. Adding Tony to the thread, but I think it is better to send the changes to upstream kernel. -- Regards Nishanth Menon OK, thanks. So I will send a series to linux (usb0 configured as peripheral for the 3 boards). Then, I will send a patch to remove usb0 node from 'am335x-evm-u-boot.dtsi'. Julien Panis
[PATCHv2 3/3] net/lwip: add lwip library for the network stack
This commit adds lwip library for the U-boot network
stack. Supported commands: ping, tftp, dhcp and wget.
Signed-off-by: Maxim Uvarov
---
.gitignore| 9 +
cmd/net.c | 38 +++-
include/net.h | 2 +-
lib/Kconfig | 2 +
lib/Makefile | 2 +
lib/lwip/Kconfig | 108 ++
lib/lwip/Makefile | 98 +
lib/lwip/apps/dhcp/lwip-dhcp.c| 52 +
lib/lwip/apps/http/lwip-wget.c| 71 +++
lib/lwip/apps/ping/lwip_ping.c| 37
lib/lwip/apps/ping/lwip_ping.h| 24 +++
lib/lwip/apps/ping/ping.h | 35
lib/lwip/apps/tftp/lwip-tftp.c| 121 +++
lib/lwip/cmd-lwip.c | 276 ++
lib/lwip/lwipopts.h | 203 +++
lib/lwip/port/if.c| 260
lib/lwip/port/include/arch/cc.h | 46 +
lib/lwip/port/include/arch/sys_arch.h | 59 ++
lib/lwip/port/include/limits.h| 0
lib/lwip/port/sys-arch.c | 20 ++
lib/lwip/ulwip.h | 9 +
net/eth-uclass.c | 4 +-
net/net.c | 25 +++
23 files changed, 1496 insertions(+), 5 deletions(-)
create mode 100644 lib/lwip/Kconfig
create mode 100644 lib/lwip/Makefile
create mode 100644 lib/lwip/apps/dhcp/lwip-dhcp.c
create mode 100644 lib/lwip/apps/http/lwip-wget.c
create mode 100644 lib/lwip/apps/ping/lwip_ping.c
create mode 100644 lib/lwip/apps/ping/lwip_ping.h
create mode 100644 lib/lwip/apps/ping/ping.h
create mode 100644 lib/lwip/apps/tftp/lwip-tftp.c
create mode 100644 lib/lwip/cmd-lwip.c
create mode 100644 lib/lwip/lwipopts.h
create mode 100644 lib/lwip/port/if.c
create mode 100644 lib/lwip/port/include/arch/cc.h
create mode 100644 lib/lwip/port/include/arch/sys_arch.h
create mode 100644 lib/lwip/port/include/limits.h
create mode 100644 lib/lwip/port/sys-arch.c
create mode 100644 lib/lwip/ulwip.h
diff --git a/.gitignore b/.gitignore
index eb769f144c..be3676c59e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -104,3 +104,12 @@ __pycache__
# pylint files
/pylint.cur
/pylint.out/
+
+lib/lwip/lwip-external
+lib/lwip/apps/ping/ping.c
+lib/lwip/apps/http/http_client.c
+lib/lwip/apps/http/http_client.h
+lib/lwip/apps/tftp/tftp.c
+lib/lwip/apps/tftp/tftp_client.h
+lib/lwip/apps/tftp/tftp_common.h
+lib/lwip/apps/tftp/tftp_example.h
diff --git a/cmd/net.c b/cmd/net.c
index 0e9f200ca9..5b3dfd6c7e 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -36,6 +36,10 @@ U_BOOT_CMD(
#endif
#ifdef CONFIG_CMD_TFTPBOOT
+#if defined(CONFIG_CMD_LWIP_REPLACE_TFTP)
+int do_lwip_tftp(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[]);
+#endif
int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
{
int ret;
@@ -56,7 +60,12 @@ U_BOOT_CMD(
);
#else
U_BOOT_CMD(
- tftpboot, 3, 1, do_tftpb,
+ tftpboot, 3, 1,
+#if defined(CONFIG_CMD_LWIP_REPLACE_TFTP)
+ do_lwip_tftp,
+#else
+ do_tftpb,
+#endif
"load file via network using TFTP protocol",
"[loadAddress] [[hostIPaddr:]bootfilename]"
);
@@ -112,7 +121,11 @@ U_BOOT_CMD(
static int do_dhcp(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
{
+#if defined(CONFIG_CMD_LWIP_REPLACE_DHCP)
+ return do_lwip_dhcp();
+#else
return netboot_common(DHCP, cmdtp, argc, argv);
+#endif
}
U_BOOT_CMD(
@@ -137,13 +150,22 @@ U_BOOT_CMD(
#endif
#if defined(CONFIG_CMD_WGET)
+#if defined(CONFIG_CMD_LWIP_REPLACE_WGET)
+int do_lwip_wget(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
+#endif
static int do_wget(struct cmd_tbl *cmdtp, int flag, int argc, char * const
argv[])
{
return netboot_common(WGET, cmdtp, argc, argv);
}
U_BOOT_CMD(
- wget, 3, 1, do_wget,
+ wget, 3, 1,
+#if defined(CONFIG_CMD_LWIP_REPLACE_WGET)
+ do_lwip_wget,
+#else
+ do_wget,
+#endif
"boot image via network using HTTP protocol",
"[loadAddress] [[hostIPaddr:]path and image name]"
);
@@ -376,6 +398,10 @@ static int netboot_common(enum proto_t proto, struct
cmd_tbl *cmdtp, int argc,
}
#if defined(CONFIG_CMD_PING)
+#if defined(CONFIG_CMD_LWIP_REPLACE_PING)
+extern int do_lwip_ping(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[]);
+#else
static int do_ping(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
{
@@ -395,9 +421,15 @@ static int do_ping(struct cmd_tbl *cmdtp, int flag, int
argc,
return CMD_RET_SUCCESS;
}
+#endif
U_BOOT_CMD(
- ping, 2, 1, do_ping,
+ ping, 2, 1,
+#if defined(CONFIG_CMD_LWIP_REPLACE_PING)
+ do_lwip_ping,
+#else
+
[PATCHv2 2/3] net/lwip: add README.lwip
Just add inital README.lwip doc.
Signed-off-by: Maxim Uvarov
---
doc/README.lwip | 90 +
1 file changed, 90 insertions(+)
create mode 100644 doc/README.lwip
diff --git a/doc/README.lwip b/doc/README.lwip
new file mode 100644
index 00..16f41049ab
--- /dev/null
+++ b/doc/README.lwip
@@ -0,0 +1,90 @@
+ RFC LWIP IP stack intergation for U-boot
+
+
+Reliable and bug free IP stack is usually an issue when you are trying to
write it
+from scratch. It looks like not, but when addinging new features it will be
chelledging.
+This RFC work is a demo to enable lwIP (lightweight IP) which is a widely used
open-source
+TCP/IP stack designed for embedded systems for U-boot. That will allow using
already
+written network applications for microcontrollers.
+
+LwIP license:
+lwIP is licensed under a BSD-style license: http://lwip.wikia.com/wiki/License.
+
+Main features include:
+- Protocols: IP, IPv6, ICMP, ND, MLD, UDP, TCP, IGMP, ARP, PPPoS, PPPoE
+- DHCP client, DNS client (incl. mDNS hostname resolver), AutoIP/APIPA
(Zeroconf), SNMP agent (v1, v2c, v3, private MIB support & MIB compiler)
+- APIs: specialized APIs for enhanced performance, optional Berkeley-alike
socket API
+- Extended features: IP forwarding over multiple network interfaces, TCP
congestion control, RTT estimation and fast recovery/fast retransmit
+- Addon applications: HTTP(S) server, SNTP client, SMTP(S) client, ping,
NetBIOS nameserver, mDNS responder, MQTT client, TFTP server
+
+U-boot implementation details:
+
+1. In general we can build lwIP as .a library and link it against u-boot or
compile it in
+the U-boot tree in the same way as other U-boot files. There are few reasons
why I selected
+the second variant: iwIP is very customizable with defines for features,
memory size, types of
+allocation, some internal types and platform specific code. And it was more
easy to enable/disable
+ debug which is also done with defines, and is needed periodically.
+
+2. lwIP has 2 APIs - raw mode and sequential (as lwIP names it, or socket API
as we name it in Linux).
+ This RFC implements only raw API as the proof of work.
+
+Raw IP means that the call back function for RX path is registered and will be
called when packet
+data passes the IP stack and is ready for the application.
+
+This RFC has an unmodified working ping example from lwip sources which
registeres this callback.
+ ping_pcb = raw_new(IP_PROTO_ICMP);
+ raw_recv(ping_pcb, ping_recv, NULL); <- ping_recv is app callback.
+ raw_bind(ping_pcb, IP_ADDR_ANY)
+
+Socket API also gives nice advantages due it will be easy to port linux socket
applications to u-boot.
+I.e. lwIP sockets compatible with the linux ones. But that will require RX
thread running in the background.
+So that means we need some kind of scheduler, locking and threading support or
find some other solution.
+
+3. Input and output
+
+RX packet path is injected to U-boot eth_rx() polling loop and TX patch is in
eth_send() accordingly.
+So we do not touch any drivers code and just eat packets when they are ready.
+
+4. Testing
+
+Unmodified ping example can be used. I did ping from qemu/u-boot tap device on
the host:
+
+=> lwip init
+=> lwip ping 192.168.1.2
+ping: recv 3 ms
+tcpdump on the host:
+5:09:10.925951 ARP, Request who-has 192.168.1.200 tell 192.168.1.200, length
28 15:09:12.114643 IP6 fe80::38e2:41ff:fec3:8bda > ip6-allrouters: ICMP6,
router solicitation, length 16 15:09:20.370725 ARP, Request who-has 192.168.1.2
tell 192.168.1.200, length 28 15:09:20.370740 ARP, Reply 192.168.1.2 is-at
3a:e2:41:c3:8b:da (oui Unknown), length 28 15:09:20.372789 IP 192.168.1.200 >
192.168.1.2: ICMP echo request, id 44975, seq 1, length 40 15:09:20.372810 IP
192.168.1.2 > 192.168.1.200: ICMP echo reply, id 44975, seq 1, length 40
15:09:25.426636 ARP, Request who-has 192.168.1.200 tell 192.168.1.2, length 28
15:09:25.426870 ARP, Reply 192.168.1.200 is-at f6:11:01:02:03:04 (oui Unknown),
length 28
+
+
+5. Wget example
+
+Http server has 192.168.1.2 IP address. (I did not port DNS resolving yet,
+but it's also exist in lwip.) So example just downloads some file with http
+protocol:
+
+Net: eth0: virtio-net#30
+Hit any key to stop autoboot: 0
+=> lwip init
+Starting lwIP, local interface IP is 192.168.1.200
+Initialized LWIP stack
+=> lwip wget http://192.168.1.2/
+downloading http://192.168.1.2/ to addr 0x4020
+downloaded chunk size 294, to addr 0x4020
+downloaded chunk size 318, to addr 0x40200126
+=> md 0x4020 0x40
+4020: 4f44213c 50595443 74682045 0a3e6c6d .
+40200010: 6d74683c 3c0a3e6c 64616568 743c0a3e ..Welcome to
+40200030: 6e69676e 2f3c2178 6c746974 3c0a3e65 nginx!.<
+40200040: 6c797473 200a3e65 62202020 2079646f style>.body
+40200050: 20200a7b 20202020 69772020 3a687464 {.width:
+40200060: 65353320 200a3b6d 20202020 6d202020 35em;.m
+40200070:
[PATCHv2 1/3] net/lwip: add lwip-external submodule
This commit adds the lwip library as a git submodule. I think there has to be advantages to compile lwip inside U-boot, i.e. use the same compiler and flags as the main code. One of them is LTO and the other is to enable additional debug options for network protocol during development. Also we can copy lwip library code inside U-boot, but for now I don't want to send all lwip code to the mailing list. So it's git submodule. Signed-off-by: Maxim Uvarov --- .gitmodules| 3 +++ lib/lwip/lwip-external | 1 + 2 files changed, 4 insertions(+) create mode 100644 .gitmodules create mode 16 lib/lwip/lwip-external diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 00..afc709af10 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "lib/lwip/lwip-external"] + path = lib/lwip/lwip-external + url = https://git.savannah.nongnu.org/git/lwip.git diff --git a/lib/lwip/lwip-external b/lib/lwip/lwip-external new file mode 16 index 00..3fe8d2fc43 --- /dev/null +++ b/lib/lwip/lwip-external @@ -0,0 +1 @@ +Subproject commit 3fe8d2fc43a9b69f7ed28c63d44a7744f9c0def9 -- 2.30.2
Pull request: u-boot-rockchip-20230629
Hi Tom, Please pull the fixex for rockchip platform: - rockchip inno phy fix; - pinctrl driver in SPL arort in specific case; - fix IO port voltage for rock5b-rk3588 board; CI: https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/pipelines/16732 Thanks, - Kever The following changes since commit 580eb31199be8a822e62f20965854a242f895d03: Merge branch 'riscv-fixes' of https://source.denx.de/u-boot/custodians/u-boot-riscv (2023-06-27 09:39:58 -0400) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-rockchip.git tags/u-boot-rockchip-20230629 for you to fetch changes up to d77d5301d7dc333db0425ee82499fe362abd259d: board: rockchip: rock5b-rk3588: fix description (2023-06-29 18:43:42 +0800) Eugen Hristev (2): phy: rockchip: inno-usb2: fix phy reg=0 case board: rockchip: rock5b-rk3588: fix description Jonas Karlman (1): pinctrl: rockchip: Fix Data Abort exception in SPL Ondrej Jirman (1): pinephone-pro: Fix I/O port voltage (GPIO3D4A is 1.8V) arch/arm/mach-rockchip/rk3588/Kconfig | 10 .../pinephone-pro-rk3399/pinephone-pro-rk3399.c| 6 +++-- drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 2 +- drivers/pinctrl/rockchip/pinctrl-rockchip-core.c | 28 +- 4 files changed, 16 insertions(+), 30 deletions(-)
Re: [PATCH v1] mtd: nand: raw: rockchip_nfc: copy hwecc PA data to oob_poi buffer
On 2023/6/22 21:59, Johan Jonker wrote:
Rockchip boot blocks are written per 4 x 512 byte sectors per page.
Each page must have a page address (PA) pointer in OOB to the next page.
Pages are written in a pattern depending on the NAND chip ID.
This logic used to build a page pattern table is not fully disclosed and
is not easy to fit in the MTD framework.
The formula in rk_nfc_write_page_hwecc() function is not correct.
Make hwecc and raw behavior identical.
Generate boot block page address and pattern for hwecc in user space
and copy PA data to/from the already reserved last 4 bytes before EEC
in the chip->oob_poi data layout.
Signed-off-by: Johan Jonker
Reviewed-by: Kever Yang
Thanks,
- Kever
---
drivers/mtd/nand/raw/rockchip_nfc.c | 34 ++---
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/nand/raw/rockchip_nfc.c
b/drivers/mtd/nand/raw/rockchip_nfc.c
index 5fcf6a6b..274489ec 100644
--- a/drivers/mtd/nand/raw/rockchip_nfc.c
+++ b/drivers/mtd/nand/raw/rockchip_nfc.c
@@ -525,7 +525,7 @@ static int rk_nfc_write_page_hwecc(struct mtd_info *mtd,
int pages_per_blk = mtd->erasesize / mtd->writesize;
int ret = 0, i, boot_rom_mode = 0;
dma_addr_t dma_data, dma_oob;
- u32 reg;
+ u32 tmp;
u8 *oob;
nand_prog_page_begin_op(chip, page, 0, NULL, 0);
@@ -552,6 +552,13 @@ static int rk_nfc_write_page_hwecc(struct mtd_info *mtd,
*
* 0xFF 0xFF 0xFF 0xFF | BBM OOB1 OOB2 OOB3 | ...
*
+* The code here just swaps the first 4 bytes with the last
+* 4 bytes without losing any data.
+*
+* The chip->oob_poi data layout:
+*
+*BBM OOB1 OOB2 OOB3 |..| PA0 PA1 PA2 PA3
+*
* Configure the ECC algorithm supported by the boot ROM.
*/
if (page < (pages_per_blk * rknand->boot_blks)) {
@@ -561,21 +568,17 @@ static int rk_nfc_write_page_hwecc(struct mtd_info *mtd,
}
for (i = 0; i < ecc->steps; i++) {
- if (!i) {
- reg = 0x;
- } else {
+ if (!i)
+ oob = chip->oob_poi + (ecc->steps - 1) *
NFC_SYS_DATA_SIZE;
+ else
oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE;
- reg = oob[0] | oob[1] << 8 | oob[2] << 16 |
- oob[3] << 24;
- }
- if (!i && boot_rom_mode)
- reg = (page & (pages_per_blk - 1)) * 4;
+ tmp = oob[0] | oob[1] << 8 | oob[2] << 16 | oob[3] << 24;
if (nfc->cfg->type == NFC_V9)
- nfc->oob_buf[i] = reg;
+ nfc->oob_buf[i] = tmp;
else
- nfc->oob_buf[i * (oob_step / 4)] = reg;
+ nfc->oob_buf[i * (oob_step / 4)] = tmp;
}
dma_data = dma_map_single((void *)nfc->page_buf,
@@ -720,12 +723,17 @@ static int rk_nfc_read_page_hwecc(struct mtd_info *mtd,
goto timeout_err;
}
- for (i = 1; i < ecc->steps; i++) {
- oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE;
+ for (i = 0; i < ecc->steps; i++) {
+ if (!i)
+ oob = chip->oob_poi + (ecc->steps - 1) *
NFC_SYS_DATA_SIZE;
+ else
+ oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE;
+
if (nfc->cfg->type == NFC_V9)
tmp = nfc->oob_buf[i];
else
tmp = nfc->oob_buf[i * (oob_step / 4)];
+
*oob++ = (u8)tmp;
*oob++ = (u8)(tmp >> 8);
*oob++ = (u8)(tmp >> 16);
--
2.30.2
[PATCH v4] imx: imx8mm: Add support for Mettler-Toledo snowflake board.
The board is similar to "Kontron SL i.MX8M Mini" SoM. There are two variants.
Snowflake_v2 has an external SDCard Interface. LEIG is the same, but without
display and without external SDCard Interface. Both variants support
only 1GiB of main memory.
[ bigeasy: porting and a bit of cleanup ]
Signed-off-by: Manuel Traut
Reviewed-by: Frieder Schrempf
Signed-off-by: Sebastian Andrzej Siewior
---
v3…v4:
- Added Frieder's tag
- Rebased on top of v2023.07-rc5.
v2…v3:
Addressing Frieder's comments:
- Add "kontron,imx8mm-sl" to the compatible string.
- Remove pca9450 node and pinctrl_i2c1. The included
imx8mm-kontron-sl.dtsi already provides this.
- Merge the overwritten values in dram_timing.ddrc_cfg/fsp_msg directly
into lpddr4_timing.c.
- Simplify the init in spl_dram_init() and panic() if the init failed.
- Replace a reference to SMARC with the current product name of the
SoM.
v1…v2:
- Clarify details about the board and whether or not there is an
external SD-card interface.
arch/arm/dts/Makefile |1 +
.../dts/imx8mm-mt-snowflake-v2-u-boot.dtsi| 116 ++
arch/arm/dts/imx8mm-mt-snowflake-v2.dts | 76 +
arch/arm/mach-imx/imx8m/Kconfig |8 +
board/mt/snowflake_v2/Kconfig | 18 +
board/mt/snowflake_v2/MAINTAINERS |7 +
board/mt/snowflake_v2/Makefile|9 +
board/mt/snowflake_v2/imximage.cfg|8 +
board/mt/snowflake_v2/lpddr4_timing.c | 1844 +
board/mt/snowflake_v2/snowflake_v2.c | 32 +
board/mt/snowflake_v2/snowflake_v2.env| 33 +
board/mt/snowflake_v2/spl.c | 147 ++
configs/snowflake_v2_emmcboot_defconfig | 134 ++
include/configs/snowflake_v2.h| 55 +
14 files changed, 2488 insertions(+)
create mode 100644 arch/arm/dts/imx8mm-mt-snowflake-v2-u-boot.dtsi
create mode 100644 arch/arm/dts/imx8mm-mt-snowflake-v2.dts
create mode 100644 board/mt/snowflake_v2/Kconfig
create mode 100644 board/mt/snowflake_v2/MAINTAINERS
create mode 100644 board/mt/snowflake_v2/Makefile
create mode 100644 board/mt/snowflake_v2/imximage.cfg
create mode 100644 board/mt/snowflake_v2/lpddr4_timing.c
create mode 100644 board/mt/snowflake_v2/snowflake_v2.c
create mode 100644 board/mt/snowflake_v2/snowflake_v2.env
create mode 100644 board/mt/snowflake_v2/spl.c
create mode 100644 configs/snowflake_v2_emmcboot_defconfig
create mode 100644 include/configs/snowflake_v2.h
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 480269fa60653..c0806b92c7bf4 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -981,6 +981,7 @@ dtb-$(CONFIG_ARCH_IMX8M) += \
imx8mm-icore-mx8mm-edimm2.2.dtb \
imx8mm-kontron-bl.dtb \
imx8mm-kontron-bl-osm-s.dtb \
+ imx8mm-mt-snowflake-v2.dtb \
imx8mm-mx8menlo.dtb \
imx8mm-phg.dtb \
imx8mm-venice.dtb \
diff --git a/arch/arm/dts/imx8mm-mt-snowflake-v2-u-boot.dtsi
b/arch/arm/dts/imx8mm-mt-snowflake-v2-u-boot.dtsi
new file mode 100644
index 0..d6476db292b79
--- /dev/null
+++ b/arch/arm/dts/imx8mm-mt-snowflake-v2-u-boot.dtsi
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Mettler Toledo GmbH
+ */
+
+/ {
+
+ aliases {
+ usbgadget0 =
+ };
+
+ usbg1: usbg1 {
+ u-boot,dm-spl;
+ compatible = "fsl,imx27-usb-gadget";
+ dr_mode = "peripheral";
+ chipidea,usb = <>;
+ status = "okay";
+ };
+
+ firmware {
+ optee {
+ compatible = "linaro,optee-tz";
+ method = "smc";
+ };
+ };
+};
+
+ {
+ status = "okay";
+ u-boot,dm-spl;
+ u-boot,dm-pre-reloc;
+};
+
+ {
+ status = "okay";
+ u-boot,dm-spl;
+ u-boot,dm-pre-reloc;
+};
+
+_ecspi1 {
+ u-boot,dm-spl;
+};
+
+_i2c1 {
+ u-boot,dm-spl;
+};
+
+_pmic {
+ u-boot,dm-spl;
+};
+
+_uart3 {
+ u-boot,dm-spl;
+ u-boot,dm-pre-reloc;
+};
+
+_usdhc1 {
+ u-boot,dm-spl;
+};
+
+_usdhc1_100mhz {
+ u-boot,dm-spl;
+};
+
+_usdhc1_200mhz {
+ u-boot,dm-spl;
+};
+
+ {
+ u-boot,dm-spl;
+};
+
+&{/soc@0/bus@3080/i2c@30a2/pmic@25/regulators} {
+ u-boot,dm-spl;
+};
+
+ {
+ u-boot,dm-spl;
+};
+
+ {
+ u-boot,dm-spl;
+};
+
+ {
+ u-boot,dm-spl;
+};
+
+ {
+ u-boot,dm-spl;
+};
+
+ {
+ u-boot,dm-spl;
+};
+
+ {
+ u-boot,dm-spl;
+};
+
+ {
+ u-boot,dm-spl;
+ u-boot,dm-pre-reloc;
+};
+
+ {
+ u-boot,dm-spl;
+};
+
+ {
+ u-boot,dm-spl;
+};
+
+_wdog {
+ u-boot,dm-spl;
+};
diff --git a/arch/arm/dts/imx8mm-mt-snowflake-v2.dts
b/arch/arm/dts/imx8mm-mt-snowflake-v2.dts
new file mode 100644
index 0..f56997543e6d9
--- /dev/null
+++ b/arch/arm/dts/imx8mm-mt-snowflake-v2.dts
@@ -0,0 +1,76 @@
+//
Re: [PATCH] rockchip: Restore support for boot scripts in legacy image format
Hi Jonas,
What is the legacy image format, does these boards really use than?
I think most of the board using linux distro image.
Thanks,
- Kever
On 2023/6/27 03:43, Jonas Karlman wrote:
Use of CONFIG_SPL_FIT_SIGNATURE=y cause CONFIG_LEGACY_IMAGE_FORMAT=n as
default, this prevent boot scripts in legacy image format from working
and was an unintended change in the listed fixes commits:
Wrong image format for "source" command
Add CONFIG_LEGACY_IMAGE_FORMAT=y to defconfig for affected boards to
restore support for boot scripts in legacy image format.
Fixes: 3bf8e4080763 ("board: rockchip: add Radxa ROCK5B Rk3588 board")
Fixes: cf777572ca31 ("rockchip: rockpro64: Use SDMA to boost eMMC performance")
Fixes: 6e2b8344d60c ("rockchip: rock-pi-4: Use SDMA to boost eMMC performance")
Fixes: 1bf49d5a4a7c ("rockchip: rk3566-radxa-cm3-io: Update defconfig")
Fixes: 703c170b40f2 ("rockchip: rk3568-evb: Update defconfig")
Fixes: 68000f750acd ("rockchip: rk3568-rock-3a: Update defconfig")
Fixes: 6fb02589a608 ("rockchip: rk3588-evb: Update defconfig")
Signed-off-by: Jonas Karlman
---
configs/evb-rk3568_defconfig | 1 +
configs/evb-rk3588_defconfig | 1 +
configs/radxa-cm3-io-rk3566_defconfig | 1 +
configs/rock-3a-rk3568_defconfig | 1 +
configs/rock-pi-4-rk3399_defconfig| 1 +
configs/rock5b-rk3588_defconfig | 1 +
configs/rockpro64-rk3399_defconfig| 1 +
7 files changed, 7 insertions(+)
diff --git a/configs/evb-rk3568_defconfig b/configs/evb-rk3568_defconfig
index 07819d105441..5f3fab7304c2 100644
--- a/configs/evb-rk3568_defconfig
+++ b/configs/evb-rk3568_defconfig
@@ -22,6 +22,7 @@ CONFIG_FIT=y
CONFIG_FIT_VERBOSE=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-evb.dtb"
# CONFIG_DISPLAY_CPUINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
diff --git a/configs/evb-rk3588_defconfig b/configs/evb-rk3588_defconfig
index d5f1c4b9ebc7..f49c2ca686a8 100644
--- a/configs/evb-rk3588_defconfig
+++ b/configs/evb-rk3588_defconfig
@@ -23,6 +23,7 @@ CONFIG_FIT=y
CONFIG_FIT_VERBOSE=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_OF_BOARD_SETUP=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3588-evb1-v10.dtb"
# CONFIG_DISPLAY_CPUINFO is not set
diff --git a/configs/radxa-cm3-io-rk3566_defconfig
b/configs/radxa-cm3-io-rk3566_defconfig
index 56802d85cc25..488723dfaa30 100644
--- a/configs/radxa-cm3-io-rk3566_defconfig
+++ b/configs/radxa-cm3-io-rk3566_defconfig
@@ -22,6 +22,7 @@ CONFIG_FIT=y
CONFIG_FIT_VERBOSE=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3566-radxa-cm3-io.dtb"
# CONFIG_DISPLAY_CPUINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
diff --git a/configs/rock-3a-rk3568_defconfig b/configs/rock-3a-rk3568_defconfig
index 616499f2f82b..753d03914d90 100644
--- a/configs/rock-3a-rk3568_defconfig
+++ b/configs/rock-3a-rk3568_defconfig
@@ -27,6 +27,7 @@ CONFIG_FIT=y
CONFIG_FIT_VERBOSE=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-rock-3a.dtb"
# CONFIG_DISPLAY_CPUINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
diff --git a/configs/rock-pi-4-rk3399_defconfig
b/configs/rock-pi-4-rk3399_defconfig
index 4b984adc6ef8..466868d80b03 100644
--- a/configs/rock-pi-4-rk3399_defconfig
+++ b/configs/rock-pi-4-rk3399_defconfig
@@ -20,6 +20,7 @@ CONFIG_PCI=y
CONFIG_DEBUG_UART=y
# CONFIG_ANDROID_BOOT_IMAGE is not set
CONFIG_SPL_FIT_SIGNATURE=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-rock-pi-4a.dtb"
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_MISC_INIT_R=y
diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
index c1155c20efa8..17205a56cd99 100644
--- a/configs/rock5b-rk3588_defconfig
+++ b/configs/rock5b-rk3588_defconfig
@@ -29,6 +29,7 @@ CONFIG_FIT=y
CONFIG_FIT_VERBOSE=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_OF_BOARD_SETUP=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3588-rock-5b.dtb"
# CONFIG_DISPLAY_CPUINFO is not set
diff --git a/configs/rockpro64-rk3399_defconfig
b/configs/rockpro64-rk3399_defconfig
index f41c03067903..dc4392c7451c 100644
--- a/configs/rockpro64-rk3399_defconfig
+++ b/configs/rockpro64-rk3399_defconfig
@@ -23,6 +23,7 @@ CONFIG_PCI=y
CONFIG_DEBUG_UART=y
CONFIG_LTO=y
CONFIG_SPL_FIT_SIGNATURE=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_BOOTSTAGE=y
CONFIG_BOOTSTAGE_REPORT=y
CONFIG_USE_PREBOOT=y
Re: [PATCH] board: rockchip: rock5b-rk3588: fix description
On 2023/6/23 18:05, Eugen Hristev wrote:
Update description with correct specifications
Fixes: 3bf8e4080763 ("board: rockchip: add Radxa ROCK5B Rk3588 board")
Signed-off-by: Eugen Hristev
Reviewed-by: Kever Yang
Thanks,
- Kever
---
arch/arm/mach-rockchip/rk3588/Kconfig | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-rockchip/rk3588/Kconfig
b/arch/arm/mach-rockchip/rk3588/Kconfig
index 3596b82f1f1c..b7d5f13bebbf 100644
--- a/arch/arm/mach-rockchip/rk3588/Kconfig
+++ b/arch/arm/mach-rockchip/rk3588/Kconfig
@@ -34,15 +34,15 @@ config TARGET_ROCK5B_RK3588
4x ARM Cortex-A76, 4x ARM Cortex-A55
4/8/16GB memory LPDDR4x
Mali G610MC4 GPU
- MIPI CSI 2 multiple lanes connector
+ 2x MIPI CSI 2 multiple lanes connector
eMMC module connector
uSD slot (up to 128GB)
- 2x USB 2.0, 2x USB 3.0
- 2x HDMI output, 1x HDMI input
- Ethernet port
+ 2x USB 2.0 Type-A, 2x USB 3.0 Type-A, 1x USB 3.0 Type-C
+ 2x HDMI 2.1 output, 1x micro HDMI input
+ 2.5 Gbps Ethernet port
40-pin IO header including UART, SPI, I2C and 5V DC power in
USB PD over USB Type-C
- Size: 85mm x 54mm
+ Size: 100mm x 72mm (Pico-ITX form factor)
config ROCKCHIP_BOOT_MODE_REG
default 0xfd588080
Re: [PATCH] pinctrl: rockchip: Fix Data Abort exception in SPL
On 2023/6/8 18:59, Jonas Karlman wrote:
Using CONFIG_ARMV8_SPL_EXCEPTION_VECTORS=y and CONFIG_OF_LIVE=y triggers
a Data Abort exception from unaligned memory access when the pinctrl
driver iterate node properties, e.g. for UART2 on RK3568.
setting mux of GPIO0-24 to 1
setting mux of GPIO0-24 to 1
"Synchronous Abort" handler, esr 0x9621
elr: e554 lr : e54c
x 0: 0a5c x 1: 0a5c
x 2: 0007 x 3: 0065
x 4: 0007 x 5: 00022d4e
x 6: 0a7c x 7: 000227a4
x 8: 00021cf0 x 9: 0a7c
x10: 00021cf0 x11: 00021cf0
x12: 003fda1c x13: 0007
x14: 003fd9ec x15: 0001c0ff
x16: 0700 x17: fdccd028
x18: 003fde20 x19: 0018
x20: 00020670 x21:
x22: 003fdb00 x23: 003fef90
x24: 00020688 x25:
x26: 0001 x27: 003ffc50
x28: x29: 003fda60
Code: b94083e1 97ffd508 93407c01 37f81260 (f9401038)
Resetting CPU ...
Fix this by replacing the loop to access node properties with use of
ofnode_for_each_prop instead of the current ifdef.
Also continue to next prop instead of aborting at first sign of an
unknown property.
This fixes the Data Abort exception and also pinconf of e.g. pull and
drive in SPL, e.g. for UART2 on RK3568.
setting mux of GPIO0-24 to 1
setting mux of GPIO0-24 to 1
setting pull of GPIO0-24 to 5
setting mux of GPIO0-25 to 1
setting mux of GPIO0-25 to 1
setting pull of GPIO0-25 to 5
Fixes: e7ae4cf27a6d ("pinctrl: rockchip: Add common rockchip pinctrl driver")
Signed-off-by: Jonas Karlman
Reviewed-by: Kever Yang
Thanks,
- Kever
---
.../pinctrl/rockchip/pinctrl-rockchip-core.c | 28 ---
1 file changed, 6 insertions(+), 22 deletions(-)
diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
index d9d61fdb726a..8ef089994f46 100644
--- a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
+++ b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
@@ -12,7 +12,6 @@
#include
#include
#include
-#include
#include "pinctrl-rockchip.h"
@@ -433,13 +432,7 @@ static int rockchip_pinctrl_set_state(struct udevice *dev,
int prop_len, param;
const u32 *data;
ofnode node;
-#ifdef CONFIG_OF_LIVE
- const struct device_node *np;
- struct property *pp;
-#else
- int property_offset, pcfg_node;
- const void *blob = gd->fdt_blob;
-#endif
+ struct ofprop prop;
data = dev_read_prop(config, "rockchip,pins", );
if (count < 0) {
debug("%s: bad array size %d\n", __func__, count);
@@ -473,24 +466,15 @@ static int rockchip_pinctrl_set_state(struct udevice *dev,
node = ofnode_get_by_phandle(conf);
if (!ofnode_valid(node))
return -ENODEV;
-#ifdef CONFIG_OF_LIVE
- np = ofnode_to_np(node);
- for (pp = np->properties; pp; pp = pp->next) {
- prop_name = pp->name;
- prop_len = pp->length;
- value = pp->value;
-#else
- pcfg_node = ofnode_to_offset(node);
- fdt_for_each_property_offset(property_offset, blob, pcfg_node) {
- value = fdt_getprop_by_offset(blob, property_offset,
- _name, _len);
+ ofnode_for_each_prop(prop, node) {
+ value = ofprop_get_property(, _name,
_len);
if (!value)
- return -ENOENT;
-#endif
+ continue;
+
param = rockchip_pinconf_prop_name_to_param(prop_name,
_val);
if (param < 0)
- break;
+ continue;
if (prop_len >= sizeof(fdt32_t))
arg = fdt32_to_cpu(*(fdt32_t *)value);
Re: [PATCH] phy: rockchip: inno-usb2: fix phy reg=0 case
On 2023/5/22 16:39, Eugen Hristev wrote:
The support for #address-cells=2 has a loophole: if the reg is actually 0,
but the #address-cells is actually 1, like in such case below:
syscon {
#address-cells = <1>;
phy {
reg = <0 0x10>;
};
};
then the second u32 of the 'reg' is the size, not the address.
The code should check for the parent's #address-cells value, and not
assume that if the first u32 is 0, then the #address-cells is 2, and the
reg property is something like
reg = <0 0xff00 0x10>;
Fixed this by looking for the #address-cells value and retrieving the
reg address only if this is ==2.
To avoid breaking anything I also kept the check `if reg==0` as some DT's
may have a wrong #address-cells as parent and even if this commit is
correct, it might break the existing wrong device-trees.
Fixes: d538efb9adcf ("phy: rockchip: inno-usb2: Add support #address_cells = 2")
Signed-off-by: Eugen Hristev
Reviewed-by: Kever Yang
Thanks,
- Kever
---
drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index 22e2797eea28..e5e02b6765b1 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -186,7 +186,7 @@ static int rockchip_usb2phy_probe(struct udevice *dev)
}
/* support address_cells=2 */
- if (reg == 0) {
+ if (dev_read_addr_cells(dev) == 2 && reg == 0) {
if (ofnode_read_u32_index(dev_ofnode(dev), "reg", 1, )) {
dev_err(dev, "%s must have reg[1]\n",
ofnode_get_name(dev_ofnode(dev)));
Re: [PATCH v2 5/9] pci: pcie_dw_rockchip: Hide BARs of the root complex
Hi Jonas,
Sorry for reply late for this patch.
I have check internally again for this BAR issue, the BAR alloce
fail for RC would not affect other process, and it always works on
vendor U-Boot tree.
And if we have to handle this, we can disable the BAR instead of
this workaround.
Thanks,
- Kever
On 2023/5/18 06:53, Jonas Karlman wrote:
PCI Autoconfig read the Root Complex BARs and try to claim the entire
1 GiB memory region on RK3568, leaving no space for any attached device.
With a memory region less than 1 GiB this was not a real issue:
PCI Autoconfig: Bus Memory region: [0-3eef],
PCI Autoconfig: Bus I/O region: [3ef0-3eff],
PCI Autoconfig: Found P2P bridge, device 0
PCI Autoconfig: BAR 0, Mem, size=0x4000, No room in resource, avail
start=1000 / size=3ef0, need=4000
PCI: Failed autoconfig bar 10
PCI Autoconfig: BAR 1, Mem, size=0x4000, No room in resource, avail
start=1000 / size=3ef0, need=4000
PCI: Failed autoconfig bar 14
PCI Autoconfig: ROM, size=0x1, address=0x1 bus_lower=0x2
PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x10
bus_lower=0x104000
With a memory region of the entire 1 GiB this leads to:
PCI Autoconfig: Bus Memory region: [4000-7fff],
PCI Autoconfig: Bus I/O region: [f010-f01f],
PCI Autoconfig: Found P2P bridge, device 0
PCI Autoconfig: BAR 0, Mem, size=0x4000, address=0x4000
bus_lower=0x8000
PCI Autoconfig: BAR 1, Mem, size=0x4000, No room in resource, avail
start=8000 / size=4000, need=4000
PCI: Failed autoconfig bar 14
PCI Autoconfig: ROM, size=0x1, No room in resource, avail start=8000
/ size=4000, need=1
PCI Autoconfig: BAR 0, Mem64, size=0x4000, No room in resource, avail
start=8000 / size=4000, need=4000
PCI: Failed autoconfig bar 10
After this change with a memory region of the entire 1 GiB:
PCI Autoconfig: Bus Memory region: [4000-7fff],
PCI Autoconfig: Bus I/O region: [f010-f01f],
PCI Autoconfig: Found P2P bridge, device 0
PCI Autoconfig: ROM, size=0x1, address=0x4000 bus_lower=0x4001
PCI Autoconfig: BAR 0, Mem64, size=0x4000, address=0x4010
bus_lower=0x40104000
Return an invalid value during config read of Root Complex BARs during
autoconfig to work around such issue.
Signed-off-by: Jonas Karlman
---
v2:
- Update commit message
drivers/pci/pcie_dw_rockchip.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
index 82a8b9c96e2b..f56773c2e58c 100644
--- a/drivers/pci/pcie_dw_rockchip.c
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -146,6 +146,32 @@ static inline void rk_pcie_writel_apb(struct rk_pcie
*rk_pcie, u32 reg,
__rk_pcie_write_apb(rk_pcie, rk_pcie->apb_base, reg, 0x4, val);
}
+/**
+ * The BARs of bridge should be hidden during enumeration to avoid
+ * allocation of the entire memory region by PCIe core on RK3568.
+ */
+static bool rk_pcie_hide_rc_bar(struct pcie_dw *pcie, pci_dev_t bdf,
+ uint offset)
+{
+ int bus = PCI_BUS(bdf) - pcie->first_busno;
+
+ return bus == 0 && PCI_DEV(bdf) == 0 && PCI_FUNC(bdf) == 0 &&
+ offset >= PCI_BASE_ADDRESS_0 && offset <= PCI_BASE_ADDRESS_1;
+}
+
+static int rk_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
+ uint offset, ulong *valuep,
+ enum pci_size_t size)
+{
+ struct pcie_dw *pcie = dev_get_priv(bus);
+ int ret = pcie_dw_read_config(bus, bdf, offset, valuep, size);
+
+ if (!ret && rk_pcie_hide_rc_bar(pcie, bdf, offset))
+ *valuep = pci_get_ff(size);
+
+ return ret;
+}
+
/**
* rk_pcie_configure() - Configure link capabilities and speed
*
@@ -476,7 +502,7 @@ rockchip_pcie_probe_err_init_port:
}
static const struct dm_pci_ops rockchip_pcie_ops = {
- .read_config= pcie_dw_read_config,
+ .read_config= rk_pcie_read_config,
.write_config = pcie_dw_write_config,
};
[PATCH] drivers: led: bcm6753: do not use null label to find the top
This driver considers that a node with an empty label is the top.
But the led class has changed, if a label is not provided for a led,
the label is filed with the node name. So we update this driver
to use a wrapper to manage the top led node.
Signed-off-by: Philippe Reynes
---
drivers/led/led_bcm6753.c | 114 --
1 file changed, 60 insertions(+), 54 deletions(-)
diff --git a/drivers/led/led_bcm6753.c b/drivers/led/led_bcm6753.c
index 88b650cbfc..2466d93011 100644
--- a/drivers/led/led_bcm6753.c
+++ b/drivers/led/led_bcm6753.c
@@ -174,57 +174,65 @@ static const struct led_ops bcm6753_led_ops = {
static int bcm6753_led_probe(struct udevice *dev)
{
- struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
-
- /* Top-level LED node */
- if (!uc_plat->label) {
- void __iomem *regs;
- u32 set_bits = 0;
-
- regs = dev_remap_addr(dev);
- if (!regs)
- return -EINVAL;
-
- if (dev_read_bool(dev, "brcm,serial-led-msb-first"))
- set_bits |= CLED_CTRL_SERIAL_LED_MSB_FIRST;
- if (dev_read_bool(dev, "brcm,serial-led-en-pol"))
- set_bits |= CLED_CTRL_SERIAL_LED_EN_POL;
- if (dev_read_bool(dev, "brcm,serial-led-clk-pol"))
- set_bits |= CLED_CTRL_SERIAL_LED_CLK_POL;
- if (dev_read_bool(dev, "brcm,serial-led-data-ppol"))
- set_bits |= CLED_CTRL_SERIAL_LED_DATA_PPOL;
-
- clrsetbits_32(regs + CLED_CTRL_REG, CLED_CTRL_MASK, set_bits);
- } else {
- struct bcm6753_led_priv *priv = dev_get_priv(dev);
- void __iomem *regs;
- unsigned int pin;
-
- regs = dev_remap_addr(dev_get_parent(dev));
- if (!regs)
- return -EINVAL;
-
- pin = dev_read_u32_default(dev, "reg", LEDS_MAX);
- if (pin >= LEDS_MAX)
- return -EINVAL;
-
- priv->regs = regs;
- priv->pin = pin;
-
- /* this led is managed by software */
- clrbits_32(regs + CLED_HW_LED_EN_REG, 1 << pin);
-
- /* configure the polarity */
- if (dev_read_bool(dev, "active-low"))
- clrbits_32(regs + CLED_PLED_OP_PPOL_REG, 1 << pin);
- else
- setbits_32(regs + CLED_PLED_OP_PPOL_REG, 1 << pin);
- }
+ struct bcm6753_led_priv *priv = dev_get_priv(dev);
+ void __iomem *regs;
+ unsigned int pin;
+
+ regs = dev_remap_addr(dev_get_parent(dev));
+ if (!regs)
+ return -EINVAL;
+
+ pin = dev_read_u32_default(dev, "reg", LEDS_MAX);
+ if (pin >= LEDS_MAX)
+ return -EINVAL;
+
+ priv->regs = regs;
+ priv->pin = pin;
+
+ /* this led is managed by software */
+ clrbits_32(regs + CLED_HW_LED_EN_REG, 1 << pin);
+
+ /* configure the polarity */
+ if (dev_read_bool(dev, "active-low"))
+ clrbits_32(regs + CLED_PLED_OP_PPOL_REG, 1 << pin);
+ else
+ setbits_32(regs + CLED_PLED_OP_PPOL_REG, 1 << pin);
return 0;
}
-static int bcm6753_led_bind(struct udevice *parent)
+U_BOOT_DRIVER(bcm6753_led) = {
+ .name = "bcm6753-led",
+ .id = UCLASS_LED,
+ .probe = bcm6753_led_probe,
+ .priv_auto = sizeof(struct bcm6753_led_priv),
+ .ops = _led_ops,
+};
+
+static int bcm6753_led_wrap_probe(struct udevice *dev)
+{
+ void __iomem *regs;
+ u32 set_bits = 0;
+
+ regs = dev_remap_addr(dev);
+ if (!regs)
+ return -EINVAL;
+
+ if (dev_read_bool(dev, "brcm,serial-led-msb-first"))
+ set_bits |= CLED_CTRL_SERIAL_LED_MSB_FIRST;
+ if (dev_read_bool(dev, "brcm,serial-led-en-pol"))
+ set_bits |= CLED_CTRL_SERIAL_LED_EN_POL;
+ if (dev_read_bool(dev, "brcm,serial-led-clk-pol"))
+ set_bits |= CLED_CTRL_SERIAL_LED_CLK_POL;
+ if (dev_read_bool(dev, "brcm,serial-led-data-ppol"))
+ set_bits |= CLED_CTRL_SERIAL_LED_DATA_PPOL;
+
+ clrsetbits_32(regs + CLED_CTRL_REG, CLED_CTRL_MASK, set_bits);
+
+ return 0;
+}
+
+static int bcm6753_led_wrap_bind(struct udevice *parent)
{
ofnode node;
@@ -247,12 +255,10 @@ static const struct udevice_id bcm6753_led_ids[] = {
{ /* sentinel */ }
};
-U_BOOT_DRIVER(bcm6753_led) = {
- .name = "bcm6753-led",
- .id = UCLASS_LED,
+U_BOOT_DRIVER(bcm6753_led_wrap) = {
+ .name = "bcm6753_led_wrap",
+ .id = UCLASS_NOP,
.of_match = bcm6753_led_ids,
- .bind = bcm6753_led_bind,
- .probe = bcm6753_led_probe,
- .priv_auto = sizeof(struct bcm6753_led_priv),
- .ops = _led_ops,
+ .probe = bcm6753_led_wrap_probe,
+ .bind = bcm6753_led_wrap_bind,
};
--
2.25.1
Re: [PATCH] cmd: efidebug: add missing efi_free_pool for dh subcommand
On Thu, 29 Jun 2023 at 05:14, Masahisa Kojima
wrote:
>
> This adds the missing efi_free_pool call for dh subcommand.
>
> Signed-off-by: Masahisa Kojima
> ---
> cmd/efidebug.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index 9622430c47..0be3af3e76 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -486,6 +486,7 @@ static int do_efi_show_handles(struct cmd_tbl *cmdtp, int
> flag,
> if (guidcmp(guid[j], _guid_device_path))
> printf(" %pUs\n", guid[j]);
> }
> + efi_free_pool(guid);
> }
>
> efi_free_pool(handles);
> --
> 2.34.1
>
Reviewed-by: Ilias Apalodimas
Re: [PATCH 2/5] rockchip: rk3399-pinebook-pro: Fix SPL max size and SPI flash payload offset
Hi Jonas,
On 6/29/23 00:11, Jonas Karlman wrote:
Hi Quentin,
On 2023-06-28 15:49, Quentin Schulz wrote:
Hi Jonas,
On 6/27/23 21:10, Jonas Karlman wrote:
TPL max size is limited to 184 KB, SPL is loaded to 0x0 and TF-A is
loaded to 0x4, this limit SPL max size to 256 KB. With BootRom only
reading first 2 KB per 4 KB page of SPI flash, 880 KB may be needed for
TPL+SPL in a worst-case scenario. (184 KB + 256 KB) x 2 = 880 KB
Use 0xE (896 KB) as the payload offset in SPI flash, this allows
for a payload of 3168 KB before env offset start to overlap.
Also add CONFIG_ROCKCHIP_SPI_IMAGE=y to build a bootable SPI flash
image, u-boot-rockchip-spi.bin.
Signed-off-by: Jonas Karlman
---
arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi | 4
configs/pinebook-pro-rk3399_defconfig| 4 +++-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
b/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
index ea7a5a17ae0f..88a77cad8d43 100644
--- a/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
@@ -10,10 +10,6 @@
chosen {
u-boot,spl-boot-order = "same-as-spl", , ,
};
-
- config {
- u-boot,spl-payload-offset = <0x6>; /* @ 384KB */
- };
Just a nitpick/question (and for the whole series): Is there any
specific reason for going back to the Kconfig way
(CONFIG_SYS_SPI_U_BOOT_OFFS) instead of just modifying the offset in the
-u-boot DTSI?
The main reason is that SYS_SPI_U_BOOT_OFFS is used as payload offset by
binman in rockchip-u-boot.dtsi, for building u-boot-rockchip-spi.bin.
And also to avoid any confusion on what value is used.
rockchip-u-boot.dtsi:
offset = ;
Fair point, converging towards the default for Rockchip platforms seems
like a good idea, thanks for the explanation.
On a different note, that also means that we're effectively breaking
current setups by moving the environment some other location. I cannot
talk for the maintainer(s) and user(s) but I thought it was worth
mentioning.
The environment should not be affected by these changes. All rk3399
boards using ENV_IS_IN_SPI_FLASH all use ENV_OFFSET=0x3F8000. u-boot.itb
or u-boot.img can be up to 3168 KB before it affects environment.
0x00: idbloader.img (TPL+SPL)
0x0E: u-boot.itb or u-boot.img (was before at 0x06)
0x3F8000: environment, same as before
I am not expecting any issues for users, as long as the updated
instructions are followed when updating u-boot in SPI flash, i.e.
writing u-boot-rockchip-spi.bin at start of SPI flash.
Sorry, seems like I had a brain fart yesterday and mixed SPL payload (so
U-Boot proper essentially) with U-Boot environment location. Thanks for
correcting me.
Since those values are dependent on the SoC and not on the hardware
design, shouldn't we rather hardcode those values for all RK3399
devices? i.e. set CONFIG_SYS_SPI_U_BOOT_OFFS to 0xe (it's currently
0 which is probably a bad default) and have SPL_MAX_SIZE set to 0x4?
I'm asking now because I did the maths for RK3399 Puma and we got it
wrong there too. We have TPL_MAX_SIZE set to 0x2e000 like all RK3399
boards and SPL_MAX_SIZE to 0x2e000. So, (0x2e000 * 2) * 2 = 0xb8000.
However we have u-boot,spl-payload-offset = <0x8>; so clearly there
are cases where this would be an issue.
In our case, changing the u-boot,spl-payload-offset isn't without
backward compatibility issues because we want to be able to boot U-Boot
proper from SPI but the TPL+SPL from eMMC for example (so the
u-boot,spl-payload-offset from the DT in the eMMC needs to match the
offset of U-Boot proper in the SPI). But since we are not aware of any
user using the fallback mechanism this way rather than TPL+SPL from SPI
and fallback to eMMC for U-Boot proper, we'd be fine changing it to
something safer rather than limiting the size of SPL too much.
Basically, what I am asking is whether we should make those RK3399
defaults instead of fixing them one by one in DTS/defconfigs? I also do
not know how critical this currently is, does it absolutely need to be
in this release (we're late in the rc process right now I believe?) or
can we manage to put more thoughts into making this generic for all
RK3399 (and maybe other SoCs from Rockchip)?
Otherwise, the maths sound good, so for the whole series:
Reviewed-by: Quentin Schulz
(thanks for the detailed commit log, really helps reviewing :) ).
Cheers,
Quentin
Regards,
Jonas
};
{
diff --git a/configs/pinebook-pro-rk3399_defconfig
b/configs/pinebook-pro-rk3399_defconfig
index 58a8b91aa60f..1109ebb7387b 100644
--- a/configs/pinebook-pro-rk3399_defconfig
+++ b/configs/pinebook-pro-rk3399_defconfig
@@ -12,6 +12,7 @@ CONFIG_ENV_OFFSET=0x3F8000
CONFIG_DEFAULT_DEVICE_TREE="rk3399-pinebook-pro"
CONFIG_DM_RESET=y
CONFIG_ROCKCHIP_RK3399=y
+CONFIG_ROCKCHIP_SPI_IMAGE=y
Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa
On Thu, Jun 29, 2023 at 8:49 AM Palmer Dabbelt wrote: > > On Wed, 28 Jun 2023 08:31:32 PDT (-0700), apa...@ventanamicro.com wrote: > > On Wed, Jun 28, 2023 at 7:46 PM Palmer Dabbelt wrote: > >> > >> On Tue, 27 Jun 2023 08:49:06 PDT (-0700), Palmer Dabbelt wrote: > >> > On Mon, 26 Jun 2023 23:52:06 PDT (-0700), apa...@ventanamicro.com wrote: > >> >> On Tue, Jun 27, 2023 at 1:23 AM Palmer Dabbelt > >> >> wrote: > >> >>> > >> >>> On Mon, 26 Jun 2023 10:38:43 PDT (-0700), apa...@ventanamicro.com > >> >>> wrote: > >> >>> > On Mon, Jun 26, 2023 at 3:42 PM Conor Dooley > >> >>> > wrote: > >> >>> >> > >> >>> >> intro > >> >>> >> = > >> >>> >> > >> >>> >> When the RISC-V dt-bindings were accepted upstream in Linux, the > >> >>> >> base > >> >>> >> ISA etc had yet to be ratified. By the ratification of the base ISA, > >> >>> >> incompatible changes had snuck into the specifications - for > >> >>> >> example the > >> >>> >> Zicsr and Zifencei extensions were spun out of the base ISA. > >> >>> >> > >> >>> >> Fast forward to today, and the reason for this patch. > >> >>> >> Currently the riscv,isa dt property permits only a specific subset > >> >>> >> of > >> >>> >> the ISA string - in particular it excludes version numbering. > >> >>> >> With the current constraints, it is not possible to discern whether > >> >>> >> "rv64i" means that the hart supports the fence.i instruction, for > >> >>> >> example. > >> >>> >> Future systems may choose to implement their own instruction > >> >>> >> fencing, > >> >>> >> perhaps using a vendor extension, or they may not implement the > >> >>> >> optional > >> >>> >> counter extensions. Software needs a way to determine this. > >> >>> >> > >> >>> >> versioning schemes > >> >>> >> == > >> >>> >> > >> >>> >> "Use the extension versions that are described in the ISA manual" > >> >>> >> you > >> >>> >> may say, and it's not like this has not been considered. > >> >>> >> Firstly, software that parses the riscv,isa property at runtime will > >> >>> >> need to contain a lookup table of some sort that maps arbitrary > >> >>> >> versions > >> >>> >> to versions it understands. There is not a consistent application of > >> >>> >> version number applied to extensions, with a higgledy-piggledy > >> >>> >> collection of tags, "bare" and versioned documents awaiting the > >> >>> >> reader > >> >>> >> on the "recently ratified extensions" page: > >> >>> >> https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions > >> >>> >> > >> >>> >> As an aside, and this is reflected in the patch too, since > >> >>> >> many > >> >>> >> extensions have yet to appear in a release of the ISA specs, > >> >>> >> they are defined by commits in their respective "working > >> >>> >> draft" > >> >>> >> repositories. > >> >>> >> > >> >>> >> Secondly, there is an issue of backwards compatibility, whereby > >> >>> >> allowing > >> >>> >> numbers in the ISA string, some parsers may be broken. This would > >> >>> >> require an additional property to be created to even use the > >> >>> >> versions in > >> >>> >> this manner. > >> >>> >> > >> >>> >> ~boolean properties~ string array property > >> >>> >> == > >> >>> >> > >> >>> >> If a new property is needed, the whole approach may as well be > >> >>> >> looked at > >> >>> >> from the bottom up. A string with limited character choices etc is > >> >>> >> hardly the best approach for communicating extension information to > >> >>> >> software. > >> >>> >> > >> >>> >> Switching to using properties that are defined on a per extension > >> >>> >> basis, > >> >>> >> allows us to define explicit meanings for the DT representation of > >> >>> >> each > >> >>> >> extension - rather than the current situation where different > >> >>> >> operating > >> >>> >> systems or other bits of software may impart different meanings to > >> >>> >> characters in the string. > >> >>> >> Clearly the best source of meanings is the specifications > >> >>> >> themselves, > >> >>> >> this just provides us the ability to choose at what point in time > >> >>> >> the > >> >>> >> meaning is set. If an extension changes incompatibility in the > >> >>> >> future, > >> >>> >> a new property will be required. > >> >>> >> > >> >>> >> Off-list, some of the RVI folks have committed to shoring up the > >> >>> >> wording > >> >>> >> in either the ISA specifications, the riscv-isa-manual or > >> >>> >> so that in the future, modifications to and additions or removals of > >> >>> >> features will require a new extension. Codifying that assertion > >> >>> >> somewhere would make it quite unlikely that compatibility would be > >> >>> >> broken, but we have the tools required to deal with it, if & when it > >> >>> >> crops up. > >> >>> >> It is in our collective interest, as consumers of extension > >> >>> >> meanings, to > >> >>> >> define a scheme that enforces compatibility. > >> >>> >> > >> >>>
