| Message ID | 20211222181607.1203191-10-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Various (build system) fixes | expand |
On Wed, Dec 22, 2021 at 06:16:07PM +0000, Andre Przywara wrote: > When we add the PSCI nodes to the provided DTB, we use dtc to de-compile > the blob first, then re-compile it with our nodes and properties added. > > In our input DTB the proper phandle references have already been lost, > all we see in the DTB is phandle properties in the target node, and some > numbers in the clocks and gpios properties: > =========== > clk24mhz { > compatible = "fixed-clock"; > #clock-cells = <0x00>; > clock-frequency = <0x16e3600>; > clock-output-names = "v2m:clk24mhz"; > -> phandle = <0x05>; > }; > ... > serial@90000 { > compatible = "arm,pl011", "arm,primecell"; > reg = <0x90000 0x1000>; > interrupts = <0x05>; > -> clocks = <0x05 0x05>; > clock-names = "uartclk", "apb_pclk"; > }; > =========== > dtc warns that those numbers might be wrong: > ========= > <stdin>:177.6-27: Warning (clocks_property): > /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000: > clocks: cell 0 is not a phandle reference > .... > ========= > The proper solution would be to use references (&v2m_clk24mhz) instead, > as there are in the source .dts file, but we don't have that information > anymore, and cannot easily recover it. > > To avoid the lengthy list of warnings, just drop those checks from the > dtc compilation run. This disables more checks than we want or need, but > we somewhat trust in the original DTB to be sane, so that should be > fine. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Thanks; applied. Mark. > --- > Makefile.am | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile.am b/Makefile.am > index 3d8128f..430b4a9 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile > $(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $< > > fdt.dtb: $(KERNEL_DTB) Makefile > - ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ - > + ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property - > > # The filesystem archive might not exist if INITRD is not being used > .PHONY: all clean $(FILESYSTEM) > -- > 2.25.1 >
Hi Andre, On 12/22/21 6:16 PM, Andre Przywara wrote: > When we add the PSCI nodes to the provided DTB, we use dtc to de-compile > the blob first, then re-compile it with our nodes and properties added. > > In our input DTB the proper phandle references have already been lost, > all we see in the DTB is phandle properties in the target node, and some > numbers in the clocks and gpios properties: > =========== > clk24mhz { > compatible = "fixed-clock"; > #clock-cells = <0x00>; > clock-frequency = <0x16e3600>; > clock-output-names = "v2m:clk24mhz"; > -> phandle = <0x05>; > }; > ... > serial@90000 { > compatible = "arm,pl011", "arm,primecell"; > reg = <0x90000 0x1000>; > interrupts = <0x05>; > -> clocks = <0x05 0x05>; > clock-names = "uartclk", "apb_pclk"; > }; > =========== > dtc warns that those numbers might be wrong: > ========= > <stdin>:177.6-27: Warning (clocks_property): > /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000: > clocks: cell 0 is not a phandle reference > .... > ========= > The proper solution would be to use references (&v2m_clk24mhz) instead, > as there are in the source .dts file, but we don't have that information > anymore, and cannot easily recover it. > > To avoid the lengthy list of warnings, just drop those checks from the > dtc compilation run. This disables more checks than we want or need, but > we somewhat trust in the original DTB to be sane, so that should be > fine. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > Makefile.am | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile.am b/Makefile.am > index 3d8128f..430b4a9 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile > $(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $< > > fdt.dtb: $(KERNEL_DTB) Makefile > - ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ - > + ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property - > > # The filesystem archive might not exist if INITRD is not being used > .PHONY: all clean $(FILESYSTEM) > dtc 1.4.1 complains FATAL ERROR: Unrecognized check name "clocks_property" Cheers Vladimir
On Thu, 13 Jan 2022 18:42:50 +0000 Vladimir Murzin <vladimir.murzin@arm.com> wrote: Hi Vladimir, > On 12/22/21 6:16 PM, Andre Przywara wrote: > > When we add the PSCI nodes to the provided DTB, we use dtc to de-compile > > the blob first, then re-compile it with our nodes and properties added. > > > > In our input DTB the proper phandle references have already been lost, > > all we see in the DTB is phandle properties in the target node, and some > > numbers in the clocks and gpios properties: > > =========== > > clk24mhz { > > compatible = "fixed-clock"; > > #clock-cells = <0x00>; > > clock-frequency = <0x16e3600>; > > clock-output-names = "v2m:clk24mhz"; > > -> phandle = <0x05>; > > }; > > ... > > serial@90000 { > > compatible = "arm,pl011", "arm,primecell"; > > reg = <0x90000 0x1000>; > > interrupts = <0x05>; > > -> clocks = <0x05 0x05>; > > clock-names = "uartclk", "apb_pclk"; > > }; > > =========== > > dtc warns that those numbers might be wrong: > > ========= > > <stdin>:177.6-27: Warning (clocks_property): > > /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000: > > clocks: cell 0 is not a phandle reference > > .... > > ========= > > The proper solution would be to use references (&v2m_clk24mhz) instead, > > as there are in the source .dts file, but we don't have that information > > anymore, and cannot easily recover it. > > > > To avoid the lengthy list of warnings, just drop those checks from the > > dtc compilation run. This disables more checks than we want or need, but > > we somewhat trust in the original DTB to be sane, so that should be > > fine. > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > Makefile.am | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Makefile.am b/Makefile.am > > index 3d8128f..430b4a9 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile > > $(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $< > > > > fdt.dtb: $(KERNEL_DTB) Makefile > > - ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ - > > + ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property - > > > > # The filesystem archive might not exist if INITRD is not being used > > .PHONY: all clean $(FILESYSTEM) > > > > dtc 1.4.1 complains Which distro ships this version? (distrowatch doesn't list dtc) tag v1.4.1 Tagger: David Gibson <david@gibson.dropbear.id.au> Date: Wed Nov 12 14:31:44 2014 +1100 Any chance it's just you and you can update this? It looks like the first version to support it is 1.4.5, as shipped for instance with Ubuntu 18.04. > FATAL ERROR: Unrecognized check name "clocks_property" Sigh, thanks for the heads up. I don't know if we want to blow up the Makefile with a feature test? Cheers, Andre
Hi Andre, On 1/13/22 7:50 PM, Andre Przywara wrote: > On Thu, 13 Jan 2022 18:42:50 +0000 > Vladimir Murzin <vladimir.murzin@arm.com> wrote: > > Hi Vladimir, > >> On 12/22/21 6:16 PM, Andre Przywara wrote: >>> When we add the PSCI nodes to the provided DTB, we use dtc to de-compile >>> the blob first, then re-compile it with our nodes and properties added. >>> >>> In our input DTB the proper phandle references have already been lost, >>> all we see in the DTB is phandle properties in the target node, and some >>> numbers in the clocks and gpios properties: >>> =========== >>> clk24mhz { >>> compatible = "fixed-clock"; >>> #clock-cells = <0x00>; >>> clock-frequency = <0x16e3600>; >>> clock-output-names = "v2m:clk24mhz"; >>> -> phandle = <0x05>; >>> }; >>> ... >>> serial@90000 { >>> compatible = "arm,pl011", "arm,primecell"; >>> reg = <0x90000 0x1000>; >>> interrupts = <0x05>; >>> -> clocks = <0x05 0x05>; >>> clock-names = "uartclk", "apb_pclk"; >>> }; >>> =========== >>> dtc warns that those numbers might be wrong: >>> ========= >>> <stdin>:177.6-27: Warning (clocks_property): >>> /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000: >>> clocks: cell 0 is not a phandle reference >>> .... >>> ========= >>> The proper solution would be to use references (&v2m_clk24mhz) instead, >>> as there are in the source .dts file, but we don't have that information >>> anymore, and cannot easily recover it. >>> >>> To avoid the lengthy list of warnings, just drop those checks from the >>> dtc compilation run. This disables more checks than we want or need, but >>> we somewhat trust in the original DTB to be sane, so that should be >>> fine. >>> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >>> --- >>> Makefile.am | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/Makefile.am b/Makefile.am >>> index 3d8128f..430b4a9 100644 >>> --- a/Makefile.am >>> +++ b/Makefile.am >>> @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile >>> $(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $< >>> >>> fdt.dtb: $(KERNEL_DTB) Makefile >>> - ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ - >>> + ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property - >>> >>> # The filesystem archive might not exist if INITRD is not being used >>> .PHONY: all clean $(FILESYSTEM) >>> >> >> dtc 1.4.1 complains > > Which distro ships this version? (distrowatch doesn't list dtc) > > tag v1.4.1 > Tagger: David Gibson <david@gibson.dropbear.id.au> > Date: Wed Nov 12 14:31:44 2014 +1100 > > Any chance it's just you and you can update this? It looks like the > first version to support it is 1.4.5, as shipped for instance with > Ubuntu 18.04. It is shipped as LSF module. I can try to ask for an update, but I thought that other people may run into it as well... > >> FATAL ERROR: Unrecognized check name "clocks_property" > > Sigh, thanks for the heads up. I don't know if we want to blow up the > Makefile with a feature test? I dunno, TBH. It look like warning used to be less evil than error... Cheers Vladimir > > Cheers, > Andre >
On Fri, Jan 14, 2022 at 08:35:06AM +0000, Vladimir Murzin wrote: > Hi Andre, > > On 1/13/22 7:50 PM, Andre Przywara wrote: > > On Thu, 13 Jan 2022 18:42:50 +0000 > > Vladimir Murzin <vladimir.murzin@arm.com> wrote: > > > > Hi Vladimir, > > > >> On 12/22/21 6:16 PM, Andre Przywara wrote: > >>> When we add the PSCI nodes to the provided DTB, we use dtc to de-compile > >>> the blob first, then re-compile it with our nodes and properties added. > >>> > >>> In our input DTB the proper phandle references have already been lost, > >>> all we see in the DTB is phandle properties in the target node, and some > >>> numbers in the clocks and gpios properties: > >>> =========== > >>> clk24mhz { > >>> compatible = "fixed-clock"; > >>> #clock-cells = <0x00>; > >>> clock-frequency = <0x16e3600>; > >>> clock-output-names = "v2m:clk24mhz"; > >>> -> phandle = <0x05>; > >>> }; > >>> ... > >>> serial@90000 { > >>> compatible = "arm,pl011", "arm,primecell"; > >>> reg = <0x90000 0x1000>; > >>> interrupts = <0x05>; > >>> -> clocks = <0x05 0x05>; > >>> clock-names = "uartclk", "apb_pclk"; > >>> }; > >>> =========== > >>> dtc warns that those numbers might be wrong: > >>> ========= > >>> <stdin>:177.6-27: Warning (clocks_property): > >>> /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000: > >>> clocks: cell 0 is not a phandle reference > >>> .... > >>> ========= > >>> The proper solution would be to use references (&v2m_clk24mhz) instead, > >>> as there are in the source .dts file, but we don't have that information > >>> anymore, and cannot easily recover it. > >>> > >>> To avoid the lengthy list of warnings, just drop those checks from the > >>> dtc compilation run. This disables more checks than we want or need, but > >>> we somewhat trust in the original DTB to be sane, so that should be > >>> fine. > >>> > >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> > >>> --- > >>> Makefile.am | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/Makefile.am b/Makefile.am > >>> index 3d8128f..430b4a9 100644 > >>> --- a/Makefile.am > >>> +++ b/Makefile.am > >>> @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile > >>> $(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $< > >>> > >>> fdt.dtb: $(KERNEL_DTB) Makefile > >>> - ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ - > >>> + ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property - > >>> > >>> # The filesystem archive might not exist if INITRD is not being used > >>> .PHONY: all clean $(FILESYSTEM) > >>> > >> > >> dtc 1.4.1 complains > > > > Which distro ships this version? (distrowatch doesn't list dtc) > > > > tag v1.4.1 > > Tagger: David Gibson <david@gibson.dropbear.id.au> > > Date: Wed Nov 12 14:31:44 2014 +1100 > > > > Any chance it's just you and you can update this? It looks like the > > first version to support it is 1.4.5, as shipped for instance with > > Ubuntu 18.04. > > It is shipped as LSF module. I can try to ask for an update, but I thought > that other people may run into it as well... > > > > >> FATAL ERROR: Unrecognized check name "clocks_property" > > > > Sigh, thanks for the heads up. I don't know if we want to blow up the > > Makefile with a feature test? > > I dunno, TBH. It look like warning used to be less evil than error... I agree. My preference would be to revert that for now, and consider the problem afresh. Andre, are you ok with that? Mark.
On Fri, 14 Jan 2022 10:44:55 +0000 Mark Rutland <mark.rutland@arm.com> wrote: > On Fri, Jan 14, 2022 at 08:35:06AM +0000, Vladimir Murzin wrote: > > Hi Andre, > > > > On 1/13/22 7:50 PM, Andre Przywara wrote: > > > On Thu, 13 Jan 2022 18:42:50 +0000 > > > Vladimir Murzin <vladimir.murzin@arm.com> wrote: > > > > > > Hi Vladimir, > > > > > >> On 12/22/21 6:16 PM, Andre Przywara wrote: > > >>> When we add the PSCI nodes to the provided DTB, we use dtc to de-compile > > >>> the blob first, then re-compile it with our nodes and properties added. > > >>> > > >>> In our input DTB the proper phandle references have already been lost, > > >>> all we see in the DTB is phandle properties in the target node, and some > > >>> numbers in the clocks and gpios properties: > > >>> =========== > > >>> clk24mhz { > > >>> compatible = "fixed-clock"; > > >>> #clock-cells = <0x00>; > > >>> clock-frequency = <0x16e3600>; > > >>> clock-output-names = "v2m:clk24mhz"; > > >>> -> phandle = <0x05>; > > >>> }; > > >>> ... > > >>> serial@90000 { > > >>> compatible = "arm,pl011", "arm,primecell"; > > >>> reg = <0x90000 0x1000>; > > >>> interrupts = <0x05>; > > >>> -> clocks = <0x05 0x05>; > > >>> clock-names = "uartclk", "apb_pclk"; > > >>> }; > > >>> =========== > > >>> dtc warns that those numbers might be wrong: > > >>> ========= > > >>> <stdin>:177.6-27: Warning (clocks_property): > > >>> /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000: > > >>> clocks: cell 0 is not a phandle reference > > >>> .... > > >>> ========= > > >>> The proper solution would be to use references (&v2m_clk24mhz) instead, > > >>> as there are in the source .dts file, but we don't have that information > > >>> anymore, and cannot easily recover it. > > >>> > > >>> To avoid the lengthy list of warnings, just drop those checks from the > > >>> dtc compilation run. This disables more checks than we want or need, but > > >>> we somewhat trust in the original DTB to be sane, so that should be > > >>> fine. > > >>> > > >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > >>> --- > > >>> Makefile.am | 2 +- > > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > > >>> > > >>> diff --git a/Makefile.am b/Makefile.am > > >>> index 3d8128f..430b4a9 100644 > > >>> --- a/Makefile.am > > >>> +++ b/Makefile.am > > >>> @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile > > >>> $(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $< > > >>> > > >>> fdt.dtb: $(KERNEL_DTB) Makefile > > >>> - ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ - > > >>> + ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property - > > >>> > > >>> # The filesystem archive might not exist if INITRD is not being used > > >>> .PHONY: all clean $(FILESYSTEM) > > >>> > > >> > > >> dtc 1.4.1 complains > > > > > > Which distro ships this version? (distrowatch doesn't list dtc) > > > > > > tag v1.4.1 > > > Tagger: David Gibson <david@gibson.dropbear.id.au> > > > Date: Wed Nov 12 14:31:44 2014 +1100 > > > > > > Any chance it's just you and you can update this? It looks like the > > > first version to support it is 1.4.5, as shipped for instance with > > > Ubuntu 18.04. > > > > It is shipped as LSF module. I can try to ask for an update, but I thought > > that other people may run into it as well... > > > > > > > >> FATAL ERROR: Unrecognized check name "clocks_property" > > > > > > Sigh, thanks for the heads up. I don't know if we want to blow up the > > > Makefile with a feature test? > > > > I dunno, TBH. It look like warning used to be less evil than error... Yeah, that's what I meant: Either revert it or extend the Makefile. > I agree. > > My preference would be to revert that for now, and consider the problem afresh. > Andre, are you ok with that? Sure, I don't want to break the build for people. I think kvmtool has some lightweight feature tests in its Makefile, I can try to steal some of it, and see how evil it looks. Or wait for half a year to see those older dtcs flushed out and try it again ;-) Cheers, Andre
On Fri, Jan 14, 2022 at 12:09:31PM +0000, Andre Przywara wrote: > On Fri, 14 Jan 2022 10:44:55 +0000 > Mark Rutland <mark.rutland@arm.com> wrote: > > > On Fri, Jan 14, 2022 at 08:35:06AM +0000, Vladimir Murzin wrote: > > > Hi Andre, > > > > > > On 1/13/22 7:50 PM, Andre Przywara wrote: > > > > On Thu, 13 Jan 2022 18:42:50 +0000 > > > > Vladimir Murzin <vladimir.murzin@arm.com> wrote: > > > > > > > > Hi Vladimir, > > > > > > > >> On 12/22/21 6:16 PM, Andre Przywara wrote: > > > >>> When we add the PSCI nodes to the provided DTB, we use dtc to de-compile > > > >>> the blob first, then re-compile it with our nodes and properties added. > > > >>> > > > >>> In our input DTB the proper phandle references have already been lost, > > > >>> all we see in the DTB is phandle properties in the target node, and some > > > >>> numbers in the clocks and gpios properties: > > > >>> =========== > > > >>> clk24mhz { > > > >>> compatible = "fixed-clock"; > > > >>> #clock-cells = <0x00>; > > > >>> clock-frequency = <0x16e3600>; > > > >>> clock-output-names = "v2m:clk24mhz"; > > > >>> -> phandle = <0x05>; > > > >>> }; > > > >>> ... > > > >>> serial@90000 { > > > >>> compatible = "arm,pl011", "arm,primecell"; > > > >>> reg = <0x90000 0x1000>; > > > >>> interrupts = <0x05>; > > > >>> -> clocks = <0x05 0x05>; > > > >>> clock-names = "uartclk", "apb_pclk"; > > > >>> }; > > > >>> =========== > > > >>> dtc warns that those numbers might be wrong: > > > >>> ========= > > > >>> <stdin>:177.6-27: Warning (clocks_property): > > > >>> /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000: > > > >>> clocks: cell 0 is not a phandle reference > > > >>> .... > > > >>> ========= > > > >>> The proper solution would be to use references (&v2m_clk24mhz) instead, > > > >>> as there are in the source .dts file, but we don't have that information > > > >>> anymore, and cannot easily recover it. > > > >>> > > > >>> To avoid the lengthy list of warnings, just drop those checks from the > > > >>> dtc compilation run. This disables more checks than we want or need, but > > > >>> we somewhat trust in the original DTB to be sane, so that should be > > > >>> fine. > > > >>> > > > >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > >>> --- > > > >>> Makefile.am | 2 +- > > > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > > > >>> > > > >>> diff --git a/Makefile.am b/Makefile.am > > > >>> index 3d8128f..430b4a9 100644 > > > >>> --- a/Makefile.am > > > >>> +++ b/Makefile.am > > > >>> @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile > > > >>> $(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $< > > > >>> > > > >>> fdt.dtb: $(KERNEL_DTB) Makefile > > > >>> - ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ - > > > >>> + ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property - > > > >>> > > > >>> # The filesystem archive might not exist if INITRD is not being used > > > >>> .PHONY: all clean $(FILESYSTEM) > > > >>> > > > >> > > > >> dtc 1.4.1 complains > > > > > > > > Which distro ships this version? (distrowatch doesn't list dtc) > > > > > > > > tag v1.4.1 > > > > Tagger: David Gibson <david@gibson.dropbear.id.au> > > > > Date: Wed Nov 12 14:31:44 2014 +1100 > > > > > > > > Any chance it's just you and you can update this? It looks like the > > > > first version to support it is 1.4.5, as shipped for instance with > > > > Ubuntu 18.04. > > > > > > It is shipped as LSF module. I can try to ask for an update, but I thought > > > that other people may run into it as well... > > > > > > > > > > >> FATAL ERROR: Unrecognized check name "clocks_property" > > > > > > > > Sigh, thanks for the heads up. I don't know if we want to blow up the > > > > Makefile with a feature test? > > > > > > I dunno, TBH. It look like warning used to be less evil than error... > > Yeah, that's what I meant: Either revert it or extend the Makefile. > > > I agree. > > > > My preference would be to revert that for now, and consider the problem afresh. > > Andre, are you ok with that? > > Sure, I don't want to break the build for people. > I think kvmtool has some lightweight feature tests in its Makefile, I can > try to steal some of it, and see how evil it looks. Or wait for half a > year to see those older dtcs flushed out and try it again ;-) FWIW, the feature test idea doesn't sound bad, though I beleive that for licensing reasons we cannot take that from kvmtool and would have to grow our own. Another option would be to extend FDT.pm to do the manipulation and drop the dtc dependency entirely. That would require more work, but might make it easier to do other DTB manipulation in future. For now I've applied a revert, with a link to this thread and some wording that we can reconsider this in future. Thanks, Mark.
diff --git a/Makefile.am b/Makefile.am index 3d8128f..430b4a9 100644 --- a/Makefile.am +++ b/Makefile.am @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile $(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $< fdt.dtb: $(KERNEL_DTB) Makefile - ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ - + ( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property - # The filesystem archive might not exist if INITRD is not being used .PHONY: all clean $(FILESYSTEM)
When we add the PSCI nodes to the provided DTB, we use dtc to de-compile the blob first, then re-compile it with our nodes and properties added. In our input DTB the proper phandle references have already been lost, all we see in the DTB is phandle properties in the target node, and some numbers in the clocks and gpios properties: =========== clk24mhz { compatible = "fixed-clock"; #clock-cells = <0x00>; clock-frequency = <0x16e3600>; clock-output-names = "v2m:clk24mhz"; -> phandle = <0x05>; }; ... serial@90000 { compatible = "arm,pl011", "arm,primecell"; reg = <0x90000 0x1000>; interrupts = <0x05>; -> clocks = <0x05 0x05>; clock-names = "uartclk", "apb_pclk"; }; =========== dtc warns that those numbers might be wrong: ========= <stdin>:177.6-27: Warning (clocks_property): /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000: clocks: cell 0 is not a phandle reference .... ========= The proper solution would be to use references (&v2m_clk24mhz) instead, as there are in the source .dts file, but we don't have that information anymore, and cannot easily recover it. To avoid the lengthy list of warnings, just drop those checks from the dtc compilation run. This disables more checks than we want or need, but we somewhat trust in the original DTB to be sane, so that should be fine. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)