Debugging dtoc?
Simon Glass
sjg at chromium.org
Fri Jul 30 20:55:42 CEST 2021
Hi Tom,
On Fri, 30 Jul 2021 at 11:26, Tom Rini <trini at konsulko.com> wrote:
>
> On Fri, Jul 30, 2021 at 10:59:22AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 30 Jul 2021 at 09:10, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Fri, Jul 30, 2021 at 08:44:29AM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 29 Jul 2021 at 07:22, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > On Wed, Jul 28, 2021 at 07:27:08PM -0600, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Wed, 28 Jul 2021 at 17:28, Simon Glass <sjg at chromium.org> wrote:
> > > > > > >
> > > > > > > Hi again,
> > > > > > >
> > > > > > > On Mon, 26 Jul 2021 at 08:06, Simon Glass <sjg at chromium.org> wrote:
> > > > > > > >
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Sun, 25 Jul 2021 at 15:10, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > So, I'm trying to fix the problem on am335x_evm (and some family
> > > > > > > > > configs) with needing SPL_OF_CONTROL enabled. This is mostly fine just
> > > > > > > > > enabling the option, except on am335x_evm itself, which is the
> > > > > > > > > kitchen-sink config and overflows memory. I've gone with switching to
> > > > > > > > > SPL_OF_PLATDATA there as am335x in general has all of the U_BOOT_DRVINFO
> > > > > > > > > entries it needs I believe. But, with the following patch:
> > > > > > > > >
> > > > > > > > > diff --git a/arch/arm/dts/am335x-evm-u-boot.dtsi b/arch/arm/dts/am335x-evm-u-boot.dtsi
> > > > > > > > > index 4cf5f9928d58..514f682cac99 100644
> > > > > > > > > --- a/arch/arm/dts/am335x-evm-u-boot.dtsi
> > > > > > > > > +++ b/arch/arm/dts/am335x-evm-u-boot.dtsi
> > > > > > > > > @@ -8,6 +8,7 @@
> > > > > > > > > &l4_per {
> > > > > > > > >
> > > > > > > > > segment at 300000 {
> > > > > > > > > + u-boot,dm-pre-reloc;
> > > > > > > > >
> > > > > > > > > target-module at e000 {
> > > > > > > > > u-boot,dm-pre-reloc;
> > > > > > > > > diff --git a/configs/am335x_boneblack_vboot_defconfig b/configs/am335x_boneblack_vboot_defconfig
> > > > > > > > > index a0baeec79edd..ffeefd1a0087 100644
> > > > > > > > > --- a/configs/am335x_boneblack_vboot_defconfig
> > > > > > > > > +++ b/configs/am335x_boneblack_vboot_defconfig
> > > > > > > > > @@ -31,6 +31,7 @@ CONFIG_CMD_SPL=y
> > > > > > > > > # CONFIG_CMD_SETEXPR is not set
> > > > > > > > > CONFIG_BOOTP_DNS2=y
> > > > > > > > > CONFIG_OF_CONTROL=y
> > > > > > > > > +CONFIG_SPL_OF_CONTROL=y
> > > > > > > > > CONFIG_ENV_OVERWRITE=y
> > > > > > > > > CONFIG_ENV_IS_IN_MMC=y
> > > > > > > > > CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
> > > > > > > > > diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig
> > > > > > > > > index a33efff42a74..f35b2a02f56b 100644
> > > > > > > > > --- a/configs/am335x_evm_defconfig
> > > > > > > > > +++ b/configs/am335x_evm_defconfig
> > > > > > > > > @@ -37,13 +37,16 @@ CONFIG_MTDIDS_DEFAULT="nand0=nand.0"
> > > > > > > > > CONFIG_MTDPARTS_DEFAULT="mtdparts=nand.0:128k(NAND.SPL),128k(NAND.SPL.backup1),128k(NAND.SPL.backup2),128k(NAND.SPL.backup3),256k(NAND.u-boot-spl-os),1m(NAND.u-boot),128k(NAND.u-boot-env),128k(NAND.u-boot-env.backup1),8m(NAND.kernel),-(NAND.file-system)"
> > > > > > > > > # CONFIG_SPL_EFI_PARTITION is not set
> > > > > > > > > CONFIG_OF_CONTROL=y
> > > > > > > > > +CONFIG_SPL_OF_CONTROL=y
> > > > > > > > > CONFIG_OF_LIST="am335x-evm am335x-bone am335x-boneblack am335x-evmsk am335x-bonegreen am335x-icev2 am335x-pocketbeagle"
> > > > > > > > > +CONFIG_SPL_OF_PLATDATA=y
> > > > > > > > > CONFIG_ENV_OVERWRITE=y
> > > > > > > > > CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> > > > > > > > > CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
> > > > > > > > > CONFIG_SPL_ENV_IS_NOWHERE=y
> > > > > > > > > CONFIG_VERSION_VARIABLE=y
> > > > > > > > > CONFIG_BOOTP_SEND_HOSTNAME=y
> > > > > > > > > +# CONFIG_SPL_SIMPLE_BUS is not set
> > > > > > > > > CONFIG_BOOTCOUNT_LIMIT=y
> > > > > > > > > CONFIG_CLK=y
> > > > > > > > > CONFIG_CLK_CDCE9XX=y
> > > > > > > > > diff --git a/configs/am335x_evm_spiboot_defconfig b/configs/am335x_evm_spiboot_defconfig
> > > > > > > > > index 8f0c330674a9..4a2a56a9af9e 100644
> > > > > > > > > --- a/configs/am335x_evm_spiboot_defconfig
> > > > > > > > > +++ b/configs/am335x_evm_spiboot_defconfig
> > > > > > > > > @@ -32,6 +32,7 @@ CONFIG_BOOTP_DNS2=y
> > > > > > > > > CONFIG_CMD_MTDPARTS=y
> > > > > > > > > # CONFIG_SPL_EFI_PARTITION is not set
> > > > > > > > > CONFIG_OF_CONTROL=y
> > > > > > > > > +CONFIG_SPL_OF_CONTROL=y
> > > > > > > > > CONFIG_OF_LIST="am335x-evm am335x-bone am335x-boneblack am335x-evmsk am335x-bonegreen am335x-icev2 am335x-pocketbeagle"
> > > > > > > > > CONFIG_ENV_OVERWRITE=y
> > > > > > > > > # CONFIG_ENV_IS_IN_FAT is not set
> > > > > > > > >
> > > > > > > > > I get the following failure and I don't see how to debug this:
> > > > > > > > > DTOC spl/dts/dt-plat.c
> > > > > > > > > Traceback (most recent call last):
> > > > > > > > > File "./tools/dtoc/dtoc", line 115, in <module>
> > > > > > > > > args.phase, instantiate=args.instantiate)
> > > > > > > > > File "/home/trini/work/u-boot/u-boot/tools/dtoc/../dtoc/dtb_platdata.py", line 1223, in run_steps
> > > > > > > > > outfile.method(plat)
> > > > > > > > > File "/home/trini/work/u-boot/u-boot/tools/dtoc/../dtoc/dtb_platdata.py", line 1081, in generate_plat
> > > > > > > > > self.output_node_plat(node)
> > > > > > > > > File "/home/trini/work/u-boot/u-boot/tools/dtoc/../dtoc/dtb_platdata.py", line 1023, in output_node_plat
> > > > > > > > > self._output_values(node)
> > > > > > > > > File "/home/trini/work/u-boot/u-boot/tools/dtoc/../dtoc/dtb_platdata.py", line 812, in _output_values
> > > > > > > > > self._output_prop(node, node.props[pname])
> > > > > > > > > File "/home/trini/work/u-boot/u-boot/tools/dtoc/../dtoc/dtb_platdata.py", line 798, in _output_prop
> > > > > > > > > self._output_list(node, prop)
> > > > > > > > > File "/home/trini/work/u-boot/u-boot/tools/dtoc/../dtoc/dtb_platdata.py", line 628, in _output_list
> > > > > > > > > vals.append(get_value(prop.type, val))
> > > > > > > > > File "/home/trini/work/u-boot/u-boot/tools/dtoc/../dtoc/dtb_platdata.py", line 126, in get_value
> > > > > > > > > val = '%#x' % fdt_util.fdt32_to_cpu(value)
> > > > > > > > > File "/home/trini/work/u-boot/u-boot/tools/dtoc/../dtoc/fdt_util.py", line 28, in fdt32_to_cpu
> > > > > > > > > return struct.unpack('>I', val)[0]
> > > > > > > > > TypeError: a bytes-like object is required, not 'bool'
> > > > > > > > > scripts/Makefile.spl:352: recipe for target 'spl/dts/dt-plat.c' failed
> > > > > > > > > make[1]: *** [spl/dts/dt-plat.c] Error 1
> > > > > > > > > make[1]: *** Deleting file 'spl/dts/dt-plat.c'
> > > > > > > > > Makefile:1999: recipe for target 'spl/u-boot-spl' failed
> > > > > > > > > make: *** [spl/u-boot-spl] Error 2
> > > > > > > >
> > > > > > > > That seems like a bug, where perhaps it is seeing a property with no
> > > > > > > > value so it thinks it is a bool, but then somehow tries to gets its
> > > > > > > > int value.
> > > > > > > >
> > > > > > > > I added a print to _output_list()
> > > > > > > >
> > > > > > > > else:
> > > > > > > > print('node', node.path, prop.name)
> > > > > > > > for val in prop.value:
> > > > > > > > vals.append(get_value(prop.type, val))
> > > > > > > >
> > > > > > > > and see that it is the 'ranges' property. Definitely seems like a bug
> > > > > > > > but I'll have to dig into it I think. The Prop() class selects a bool
> > > > > > > > type when the value is empty, but somehow that isn't happening.
> > > > > > >
> > > > > > > This is actually a little tricky, but I'm looking at it.
> > > > > >
> > > > > > OK I think I have figured this out. I sent a series.
> > > > >
> > > > > That gets everything building, thanks! It also shows that some parts of
> > > > > the kitchen-sink config there depend on OF_CONTROL features, really.
> > > > > What I'm going to prove out next (but will require a little bit of time
> > > > > due to having to set up lab stuff and dig out notes) is that for this
> > > > > case, we can just go with however ROM configured the PHY. We were also
> > > > > still over SRAM space, with PLATDATA enabled too.
> > > >
> > > > OK good. I think more users would help reduce the number of rough edges .
> > > >
> > > > Can you please point me to the tree with before/after? I would like to
> > > > look at SRAM space just in case I can see something.
> > >
> > > With SPL_OF_CONTROL, we're 4900 bytes over (and it's just adding
> > > SPL_OF_CONTROL to the current config). PLATDATA isn't an option since
> > > both omap_hsmmc.c and drivers/net/ti/cpsw.c need gpio by name/etc
> >
> > Actually we can handle this - see for example how
> > clk_get_by_driver_info() works.
> >
> > > functionality. The mmc case could be kludged (assume card is writable
> > > if needed) but cpsw is harder at first glance.
> >
> > I think we should figure out how to enable of-platdata on this board.
>
> The MMC case can be fixed a few ways. It's "find the GPIO for
> card-change or write-protect" and just leaving that in the state ROM has
> it, is fine here. The driver can really just be built with this off.
> The network driver seems a bit more complex.
> drivers/net/ti/cpsw-common.c::ti_cm_get_macid_addr() and similar need to
> workout with PLATDATA instead. For that case where we can use one SPL
> and one DTB to boot a platform (or, family of platforms), something
> could be worked out so that we figure it all out at build-time what path
> will be taken. But the CPSW driver is an example of something that
> fairly mirrors the linux kernel driver and does as much via the DT as
> possible. I'm not sure how we rework this.
I'll have a look too. If we cannot make of-platdata work in all
reasonable situations, then it isn't really fair to ask people to use
it.
Regards,
Simon
More information about the U-Boot
mailing list