| Message ID | 20170721191014.8161-2-robdclark@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On Fri, Jul 21, 2017 at 03:10:10PM -0400, Rob Clark wrote: > Snapdragon is a bit of a funny case, where we want to build the fdt to > pass to lk, which loads us, but otherwise want to treat it as OF_BOARD, > since lk will patch the fdt (for example, to insert simple-framebuffer > node). > > Signed-off-by: Rob Clark <robdclark@gmail.com> > --- > Makefile | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Makefile b/Makefile > index 452596485d..0749cdfc5d 100644 > --- a/Makefile > +++ b/Makefile > @@ -780,6 +780,12 @@ ALL-$(CONFIG_SPL_FRAMEWORK) += u-boot.img > endif > ALL-$(CONFIG_TPL) += tpl/u-boot-tpl.bin > ALL-$(CONFIG_OF_SEPARATE) += u-boot.dtb > +# Snapdragon devices, where lk/aboot loads u-boot is kind of a special > +# case, because lk loads the fdt which is embedded (along with u-boot) > +# in a boot.img. But it also does some fdt fixups. So generally we > +# want to treat it like CONFIG_OF_BOARD=y, except that we also want > +# to build the dtb's > +ALL-$(CONFIG_ARCH_SNAPDRAGON) += u-boot.dtb > ifeq ($(CONFIG_SPL_FRAMEWORK),y) > ALL-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img > endif We really can't just select OF_SEPARATE here? Because the code in lib/fdtdec.c is wrong, as it stands, yes? If so, could we shuffle that code right to allow for the case of "we get our DT modified by firmware that loads us" ? That doesn't soune like a super uncommon case moving forward.
On Fri, Jul 21, 2017 at 6:18 PM, Tom Rini <trini@konsulko.com> wrote: > On Fri, Jul 21, 2017 at 03:10:10PM -0400, Rob Clark wrote: > >> Snapdragon is a bit of a funny case, where we want to build the fdt to >> pass to lk, which loads us, but otherwise want to treat it as OF_BOARD, >> since lk will patch the fdt (for example, to insert simple-framebuffer >> node). >> >> Signed-off-by: Rob Clark <robdclark@gmail.com> >> --- >> Makefile | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/Makefile b/Makefile >> index 452596485d..0749cdfc5d 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -780,6 +780,12 @@ ALL-$(CONFIG_SPL_FRAMEWORK) += u-boot.img >> endif >> ALL-$(CONFIG_TPL) += tpl/u-boot-tpl.bin >> ALL-$(CONFIG_OF_SEPARATE) += u-boot.dtb >> +# Snapdragon devices, where lk/aboot loads u-boot is kind of a special >> +# case, because lk loads the fdt which is embedded (along with u-boot) >> +# in a boot.img. But it also does some fdt fixups. So generally we >> +# want to treat it like CONFIG_OF_BOARD=y, except that we also want >> +# to build the dtb's >> +ALL-$(CONFIG_ARCH_SNAPDRAGON) += u-boot.dtb >> ifeq ($(CONFIG_SPL_FRAMEWORK),y) >> ALL-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img >> endif > > We really can't just select OF_SEPARATE here? Because the code in > lib/fdtdec.c is wrong, as it stands, yes? If so, could we shuffle that > code right to allow for the case of "we get our DT modified by firmware > that loads us" ? That doesn't soune like a super uncommon case moving > forward. from a quick look, I think the only reason we need OF_BOARD is the call to board_fdt_blob_setup() in fdtdec.. I guess we could make that a weak sym, and call it unconditionally.. in that case I think OF_SEPARATE would work. If you think that is a cleaner solution, I can give that a try. BR, -R
On Fri, Jul 21, 2017 at 06:57:58PM -0400, Rob Clark wrote: > On Fri, Jul 21, 2017 at 6:18 PM, Tom Rini <trini@konsulko.com> wrote: > > On Fri, Jul 21, 2017 at 03:10:10PM -0400, Rob Clark wrote: > > > >> Snapdragon is a bit of a funny case, where we want to build the fdt to > >> pass to lk, which loads us, but otherwise want to treat it as OF_BOARD, > >> since lk will patch the fdt (for example, to insert simple-framebuffer > >> node). > >> > >> Signed-off-by: Rob Clark <robdclark@gmail.com> > >> --- > >> Makefile | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/Makefile b/Makefile > >> index 452596485d..0749cdfc5d 100644 > >> --- a/Makefile > >> +++ b/Makefile > >> @@ -780,6 +780,12 @@ ALL-$(CONFIG_SPL_FRAMEWORK) += u-boot.img > >> endif > >> ALL-$(CONFIG_TPL) += tpl/u-boot-tpl.bin > >> ALL-$(CONFIG_OF_SEPARATE) += u-boot.dtb > >> +# Snapdragon devices, where lk/aboot loads u-boot is kind of a special > >> +# case, because lk loads the fdt which is embedded (along with u-boot) > >> +# in a boot.img. But it also does some fdt fixups. So generally we > >> +# want to treat it like CONFIG_OF_BOARD=y, except that we also want > >> +# to build the dtb's > >> +ALL-$(CONFIG_ARCH_SNAPDRAGON) += u-boot.dtb > >> ifeq ($(CONFIG_SPL_FRAMEWORK),y) > >> ALL-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img > >> endif > > > > We really can't just select OF_SEPARATE here? Because the code in > > lib/fdtdec.c is wrong, as it stands, yes? If so, could we shuffle that > > code right to allow for the case of "we get our DT modified by firmware > > that loads us" ? That doesn't soune like a super uncommon case moving > > forward. > > from a quick look, I think the only reason we need OF_BOARD is the > call to board_fdt_blob_setup() in fdtdec.. > > I guess we could make that a weak sym, and call it unconditionally.. > in that case I think OF_SEPARATE would work. If you think that is a > cleaner solution, I can give that a try. I think so. I really don't want to have changes in the Makefiles to special case logic like this if we can avoid it, without making the code ugly too. Thanks!
diff --git a/Makefile b/Makefile index 452596485d..0749cdfc5d 100644 --- a/Makefile +++ b/Makefile @@ -780,6 +780,12 @@ ALL-$(CONFIG_SPL_FRAMEWORK) += u-boot.img endif ALL-$(CONFIG_TPL) += tpl/u-boot-tpl.bin ALL-$(CONFIG_OF_SEPARATE) += u-boot.dtb +# Snapdragon devices, where lk/aboot loads u-boot is kind of a special +# case, because lk loads the fdt which is embedded (along with u-boot) +# in a boot.img. But it also does some fdt fixups. So generally we +# want to treat it like CONFIG_OF_BOARD=y, except that we also want +# to build the dtb's +ALL-$(CONFIG_ARCH_SNAPDRAGON) += u-boot.dtb ifeq ($(CONFIG_SPL_FRAMEWORK),y) ALL-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img endif
Snapdragon is a bit of a funny case, where we want to build the fdt to pass to lk, which loads us, but otherwise want to treat it as OF_BOARD, since lk will patch the fdt (for example, to insert simple-framebuffer node). Signed-off-by: Rob Clark <robdclark@gmail.com> --- Makefile | 6 ++++++ 1 file changed, 6 insertions(+)