[PATCH] scripts/Makefile.lib: add -D__U_BOOT__ to dtc_cpp_flags

[Tom Rini]

On Mon, Jul 04, 2022 at 12:59:58PM +0200, Rasmus Villemoes wrote:
> On 01/07/2022 21.25, Tom Rini wrote:
> > On Fri, Jul 01, 2022 at 09:27:59AM +0200, Rasmus Villemoes wrote:
> >> When trying to use the exact same device tree source to build the dtbs
> >> used with U-Boot and linux, one often runs into various problems. For
> >> example, files under include/dt-bindings/ might not be completely in
> >> sync, so some #defines are available in linux but not in U-Boot.
> >>
> >> It's also possible that there are nodes/hardware one simply doesn't
> >> need or want to describe to U-Boot, e.g. to reduce the size of the
> >> .dtb (for SPL, there's already the whole fdtgrep machinery to deal
> >> with that, but not for U-Boot proper), or configuration settings like
> >> watchdog timeout one wants to make different between linux and U-Boot.
> >>
> >> So in order to allow such tweaks, add an explicit define of the macro
> >> __U_BOOT__. Since U-Boot includes linux/kconfig.h (and defines
> >> __ASSEMBLY__ to make that work) while linux doesn't, one could also
> >> use __ASSEMBLY__ or just about any known-to-be-defined CONFIG_* macro
> >> as a proxy, but it's much more readable, maintainable and robust if
> >> one's .dts has "ifdef __U_BOOT__" rather than "ifdef __ASSEMBLY__".
> >>
> >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> >> ---
> >>  scripts/Makefile.lib | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> >> index 3db2550085..4183767a62 100644
> >> --- a/scripts/Makefile.lib
> >> +++ b/scripts/Makefile.lib
> >> @@ -190,6 +190,7 @@ dtc_cpp_flags  = -Wp,-MD,$(depfile).pre.tmp -nostdinc                    \
> >>  		 -I$(srctree)/arch/$(ARCH)/dts/include                   \
> >>  		 -I$(srctree)/include                                    \
> >>  		 -D__ASSEMBLY__                                          \
> >> +		 -D__U_BOOT__                                            \
> >>  		 -undef -D__DTS__
> >>  
> >>  # Finds the multi-part object the current object will be linked into
> > 
> > Since this implies to me using __U_BOOT__ in the main dts file itself,
> > we need to bring this up on devicetree-spec.  Otherwise, we could just
> > use the -u-boot.dtsi to remove nodes/properties, yes? 
> 
> Kind of, but not totally. For example, one snippet I have in a board .dts is
> 
> #ifdef __U_BOOT__
> #include "imx8mp.dtsi"
> #else
> #include "../freescale/imx8mp.dtsi"
> #endif
> 
> because U-Boot doesn't divide board dts in vendor dirs, but linux does -
> and when one doesn't want to put the linux .dts in the freescale dir
> (because it's not a freescale board, just based on a freescale SOC - and
> I want to be able to maintain this going forward, which means I don't
> want to resolve trivial merge conflicts in the freescale/Makefile over
> and over). And anything else which is purely cpp stuff, e.g. relying on
> dt-bindings/foo.h providing definitions of certain macros, cannot be
> worked around with -u-boot.dtsi.

I'm trying to not get lost in specifics here.  Figuring out a better way
to keep our copies of the dts files in sync is needed because yes, we
need to mirror the directory structure too.  So, sigh, many of our dts
files need to be shuffled around.

> > And then bindings
> > needing to be re-synced is something to incentivize, from my point of
> > view, initially at least.
> 
> No doubt about that. It's just that when the version of U-Boot one is
> currently based on doesn't have some include/dt-bindings/whatnot.h or it
> lacks some defines, and whatnot is not essential for the bootloader
> stage, it's much easier to just elide the nodes (in my case some RGB LED
> stuff) than to try to carry a patch updating whatnot.h, only to find
> that it conflicts with some newer U-Boot version a month later.
> 
> It's also somewhat convoluted to add some node or property in the main
> .dts file, only to override that in a -u-boot.dtsi file; it's easier to
> understand what's going on if one can directly see a #ifdef __U_BOOT__.
> 
> But, as I said, one can use any known-to-be-defined CONFIG_ thing as a
> proxy, and I could just do
> 
> #ifdef CONFIG_SYS_VENDOR
> #define __U_BOOT__
> #endif
> 
> at the very top of my .dts file, or maintain this patch as-is
> out-of-tree. I just thought it might be useful for others. If not, feel
> free to drop it.

Well, setting aside the whole sync question, do you believe you can get
__U_BOOT__ or whatever else accepted upstream, or are you expecting to
keep track of the differences when you resync your boards?  At the end
of the day, I want to make things easier, not harder, on developers to
keep the files up to date and correct, and I gather that I'm not quite
seeing the use cases / problems correctly right now.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220705/82bf73d1/attachment.sig>