[boot-wrapper PATCH v2 9/9] avoid dtc warnings on re-compiling DTB

[Andre Przywara]

On Fri, 14 Jan 2022 10:44:55 +0000
Mark Rutland <mark.rutland at 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 at 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 at 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 at 8000000/motherboard-bus at 8000000/iofpga-bus at 300000000/serial at 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 at 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 at 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