| Message ID | 20200417072137.801234-4-masahiroy@kernel.org |
|---|---|
| State | Accepted |
| Commit | f34d0ad82e51df9dbd5f80bc0f17c3542de2bc0e |
| Headers | show |
| Series | support separate asm-offsets.h for SPL and TPL | expand |
On Fri, Apr 17, 2020 at 3:22 PM Masahiro Yamada <masahiroy at kernel.org> wrote: > > Currently generic-asm-offsets.h and asm-offsets.h are generated based > on U-Boot proper config options. The same asm-offsets headers are used > for building U-Boot SPL/TPL, which causes potential offset mismatch if > U-Boot proper has different config options from U-Boot SPL/TPL. > > This commit adds: > spl/include/generated/(generic-)asm-offsets.h > tpl/include/generated/(generic-)asm-offsets.h > > spl/include/generated/(generic-)asm-offsets.h is generated if > CONFIG_SPL=y, and included when building SPL. > > tpl/include/generated/(generic-)asm-offsets.h is generated if > CONFIG_TPL=y, and included when building TPL. > > They are created before Kbuild descends into SPL/TPL object directories > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c > includes a bunch of headers. > > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h > is searched in {spl,tpl}/include/generated/. > > Requested-by: Bin Meng <bmeng.cn at gmail.com> > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org> > --- > > Changes in v2: None > > Kbuild | 4 ++-- > scripts/Makefile.spl | 10 ++++++++-- > 2 files changed, 10 insertions(+), 4 deletions(-) > It works like a charm. Thanks! Tested-by: Bin Meng <bmeng.cn at gmail.com>
Hi Tom, On Sat, Apr 18, 2020 at 8:37 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > On Fri, Apr 17, 2020 at 3:22 PM Masahiro Yamada <masahiroy at kernel.org> wrote: > > > > Currently generic-asm-offsets.h and asm-offsets.h are generated based > > on U-Boot proper config options. The same asm-offsets headers are used > > for building U-Boot SPL/TPL, which causes potential offset mismatch if > > U-Boot proper has different config options from U-Boot SPL/TPL. > > > > This commit adds: > > spl/include/generated/(generic-)asm-offsets.h > > tpl/include/generated/(generic-)asm-offsets.h > > > > spl/include/generated/(generic-)asm-offsets.h is generated if > > CONFIG_SPL=y, and included when building SPL. > > > > tpl/include/generated/(generic-)asm-offsets.h is generated if > > CONFIG_TPL=y, and included when building TPL. > > > > They are created before Kbuild descends into SPL/TPL object directories > > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c > > includes a bunch of headers. > > > > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h > > is searched in {spl,tpl}/include/generated/. > > > > Requested-by: Bin Meng <bmeng.cn at gmail.com> > > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org> > > --- > > > > Changes in v2: None > > > > Kbuild | 4 ++-- > > scripts/Makefile.spl | 10 ++++++++-- > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > It works like a charm. Thanks! > > Tested-by: Bin Meng <bmeng.cn at gmail.com> It looks only patch 1 and 2 in this series were applied. This patch was missed. Regards, Bin
On Mon, Apr 27, 2020 at 09:37:28AM +0800, Bin Meng wrote: > Hi Tom, > > On Sat, Apr 18, 2020 at 8:37 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > On Fri, Apr 17, 2020 at 3:22 PM Masahiro Yamada <masahiroy at kernel.org> wrote: > > > > > > Currently generic-asm-offsets.h and asm-offsets.h are generated based > > > on U-Boot proper config options. The same asm-offsets headers are used > > > for building U-Boot SPL/TPL, which causes potential offset mismatch if > > > U-Boot proper has different config options from U-Boot SPL/TPL. > > > > > > This commit adds: > > > spl/include/generated/(generic-)asm-offsets.h > > > tpl/include/generated/(generic-)asm-offsets.h > > > > > > spl/include/generated/(generic-)asm-offsets.h is generated if > > > CONFIG_SPL=y, and included when building SPL. > > > > > > tpl/include/generated/(generic-)asm-offsets.h is generated if > > > CONFIG_TPL=y, and included when building TPL. > > > > > > They are created before Kbuild descends into SPL/TPL object directories > > > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c > > > includes a bunch of headers. > > > > > > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h > > > is searched in {spl,tpl}/include/generated/. > > > > > > Requested-by: Bin Meng <bmeng.cn at gmail.com> > > > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org> > > > --- > > > > > > Changes in v2: None > > > > > > Kbuild | 4 ++-- > > > scripts/Makefile.spl | 10 ++++++++-- > > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > > > > It works like a charm. Thanks! > > > > Tested-by: Bin Meng <bmeng.cn at gmail.com> > > It looks only patch 1 and 2 in this series were applied. This patch was missed. So, I was going to ask and forgot, sorry. Why do we need to have these differ between TPL/SPL/U-Boot itself, and then why is that good? I thought this was something we wanted to ensure was consistent from stage to stage to facilitate sharing of information. But I also am happy to assume I just missed an explanation along the way. Thanks!
Hi Tom, On Mon, Apr 27, 2020 at 9:37 PM Tom Rini <trini at konsulko.com> wrote: > > On Mon, Apr 27, 2020 at 09:37:28AM +0800, Bin Meng wrote: > > Hi Tom, > > > > On Sat, Apr 18, 2020 at 8:37 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > > > On Fri, Apr 17, 2020 at 3:22 PM Masahiro Yamada <masahiroy at kernel.org> wrote: > > > > > > > > Currently generic-asm-offsets.h and asm-offsets.h are generated based > > > > on U-Boot proper config options. The same asm-offsets headers are used > > > > for building U-Boot SPL/TPL, which causes potential offset mismatch if > > > > U-Boot proper has different config options from U-Boot SPL/TPL. > > > > > > > > This commit adds: > > > > spl/include/generated/(generic-)asm-offsets.h > > > > tpl/include/generated/(generic-)asm-offsets.h > > > > > > > > spl/include/generated/(generic-)asm-offsets.h is generated if > > > > CONFIG_SPL=y, and included when building SPL. > > > > > > > > tpl/include/generated/(generic-)asm-offsets.h is generated if > > > > CONFIG_TPL=y, and included when building TPL. > > > > > > > > They are created before Kbuild descends into SPL/TPL object directories > > > > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c > > > > includes a bunch of headers. > > > > > > > > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h > > > > is searched in {spl,tpl}/include/generated/. > > > > > > > > Requested-by: Bin Meng <bmeng.cn at gmail.com> > > > > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org> > > > > --- > > > > > > > > Changes in v2: None > > > > > > > > Kbuild | 4 ++-- > > > > scripts/Makefile.spl | 10 ++++++++-- > > > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > > > > > > > It works like a charm. Thanks! > > > > > > Tested-by: Bin Meng <bmeng.cn at gmail.com> > > > > It looks only patch 1 and 2 in this series were applied. This patch was missed. > > So, I was going to ask and forgot, sorry. Why do we need to have these > differ between TPL/SPL/U-Boot itself, and then why is that good? I Because currently the offsets {spl,tpl}/include/generated/(generic-)asm-offsets.h are generated per the U-Boot config options. But SPL and TPL are not guaranteed to have the same config options. For example it's possible to have SPL/TPL turned on {SPL,TPL}_CONFIG_XXX but U-Boot to turn off CONFIG_XXX, or vice verse. To me this is more like a bug fix other than a feature. > thought this was something we wanted to ensure was consistent from stage > to stage to facilitate sharing of information. But I also am happy to > assume I just missed an explanation along the way. Thanks! Regards, Bin
On Mon, Apr 27, 2020 at 10:50:48PM +0800, Bin Meng wrote: > Hi Tom, > > On Mon, Apr 27, 2020 at 9:37 PM Tom Rini <trini at konsulko.com> wrote: > > > > On Mon, Apr 27, 2020 at 09:37:28AM +0800, Bin Meng wrote: > > > Hi Tom, > > > > > > On Sat, Apr 18, 2020 at 8:37 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > > > > > On Fri, Apr 17, 2020 at 3:22 PM Masahiro Yamada <masahiroy at kernel.org> wrote: > > > > > > > > > > Currently generic-asm-offsets.h and asm-offsets.h are generated based > > > > > on U-Boot proper config options. The same asm-offsets headers are used > > > > > for building U-Boot SPL/TPL, which causes potential offset mismatch if > > > > > U-Boot proper has different config options from U-Boot SPL/TPL. > > > > > > > > > > This commit adds: > > > > > spl/include/generated/(generic-)asm-offsets.h > > > > > tpl/include/generated/(generic-)asm-offsets.h > > > > > > > > > > spl/include/generated/(generic-)asm-offsets.h is generated if > > > > > CONFIG_SPL=y, and included when building SPL. > > > > > > > > > > tpl/include/generated/(generic-)asm-offsets.h is generated if > > > > > CONFIG_TPL=y, and included when building TPL. > > > > > > > > > > They are created before Kbuild descends into SPL/TPL object directories > > > > > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c > > > > > includes a bunch of headers. > > > > > > > > > > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h > > > > > is searched in {spl,tpl}/include/generated/. > > > > > > > > > > Requested-by: Bin Meng <bmeng.cn at gmail.com> > > > > > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org> > > > > > --- > > > > > > > > > > Changes in v2: None > > > > > > > > > > Kbuild | 4 ++-- > > > > > scripts/Makefile.spl | 10 ++++++++-- > > > > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > > > > > > > > > > It works like a charm. Thanks! > > > > > > > > Tested-by: Bin Meng <bmeng.cn at gmail.com> > > > > > > It looks only patch 1 and 2 in this series were applied. This patch was missed. > > > > So, I was going to ask and forgot, sorry. Why do we need to have these > > differ between TPL/SPL/U-Boot itself, and then why is that good? I > > Because currently the offsets > {spl,tpl}/include/generated/(generic-)asm-offsets.h are generated per > the U-Boot config options. But SPL and TPL are not guaranteed to have > the same config options. > > For example it's possible to have SPL/TPL turned on > {SPL,TPL}_CONFIG_XXX but U-Boot to turn off CONFIG_XXX, or vice verse. > > To me this is more like a bug fix other than a feature. Ah, OK. But shouldn't we make sure that asm-offsets are consistent between SPL/TPL/U-Boot? Or is there a use-case where it makes sense for them to differ intentionally?
Hi Tom, On Tue, Apr 28, 2020 at 10:46 PM Tom Rini <trini at konsulko.com> wrote: > > On Mon, Apr 27, 2020 at 10:50:48PM +0800, Bin Meng wrote: > > Hi Tom, > > > > On Mon, Apr 27, 2020 at 9:37 PM Tom Rini <trini at konsulko.com> wrote: > > > > > > On Mon, Apr 27, 2020 at 09:37:28AM +0800, Bin Meng wrote: > > > > Hi Tom, > > > > > > > > On Sat, Apr 18, 2020 at 8:37 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > > > > > > > On Fri, Apr 17, 2020 at 3:22 PM Masahiro Yamada <masahiroy at kernel.org> wrote: > > > > > > > > > > > > Currently generic-asm-offsets.h and asm-offsets.h are generated based > > > > > > on U-Boot proper config options. The same asm-offsets headers are used > > > > > > for building U-Boot SPL/TPL, which causes potential offset mismatch if > > > > > > U-Boot proper has different config options from U-Boot SPL/TPL. > > > > > > > > > > > > This commit adds: > > > > > > spl/include/generated/(generic-)asm-offsets.h > > > > > > tpl/include/generated/(generic-)asm-offsets.h > > > > > > > > > > > > spl/include/generated/(generic-)asm-offsets.h is generated if > > > > > > CONFIG_SPL=y, and included when building SPL. > > > > > > > > > > > > tpl/include/generated/(generic-)asm-offsets.h is generated if > > > > > > CONFIG_TPL=y, and included when building TPL. > > > > > > > > > > > > They are created before Kbuild descends into SPL/TPL object directories > > > > > > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c > > > > > > includes a bunch of headers. > > > > > > > > > > > > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h > > > > > > is searched in {spl,tpl}/include/generated/. > > > > > > > > > > > > Requested-by: Bin Meng <bmeng.cn at gmail.com> > > > > > > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org> > > > > > > --- > > > > > > > > > > > > Changes in v2: None > > > > > > > > > > > > Kbuild | 4 ++-- > > > > > > scripts/Makefile.spl | 10 ++++++++-- > > > > > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > It works like a charm. Thanks! > > > > > > > > > > Tested-by: Bin Meng <bmeng.cn at gmail.com> > > > > > > > > It looks only patch 1 and 2 in this series were applied. This patch was missed. > > > > > > So, I was going to ask and forgot, sorry. Why do we need to have these > > > differ between TPL/SPL/U-Boot itself, and then why is that good? I > > > > Because currently the offsets > > {spl,tpl}/include/generated/(generic-)asm-offsets.h are generated per > > the U-Boot config options. But SPL and TPL are not guaranteed to have > > the same config options. > > > > For example it's possible to have SPL/TPL turned on > > {SPL,TPL}_CONFIG_XXX but U-Boot to turn off CONFIG_XXX, or vice verse. > > > > To me this is more like a bug fix other than a feature. > > Ah, OK. But shouldn't we make sure that asm-offsets are consistent > between SPL/TPL/U-Boot? Or is there a use-case where it makes sense for > them to differ intentionally? I don't know other boards, but for now RISC-V does turn on SPL_SMP in SPL, but does not turn on SMP in U-Boot S-mode. U-Boot M-mode can still turn on SMP. Regards, Bin
On Tue, Apr 28, 2020 at 11:01:12PM +0800, Bin Meng wrote: > Hi Tom, > > On Tue, Apr 28, 2020 at 10:46 PM Tom Rini <trini at konsulko.com> wrote: > > > > On Mon, Apr 27, 2020 at 10:50:48PM +0800, Bin Meng wrote: > > > Hi Tom, > > > > > > On Mon, Apr 27, 2020 at 9:37 PM Tom Rini <trini at konsulko.com> wrote: > > > > > > > > On Mon, Apr 27, 2020 at 09:37:28AM +0800, Bin Meng wrote: > > > > > Hi Tom, > > > > > > > > > > On Sat, Apr 18, 2020 at 8:37 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > > > > > > > > > On Fri, Apr 17, 2020 at 3:22 PM Masahiro Yamada <masahiroy at kernel.org> wrote: > > > > > > > > > > > > > > Currently generic-asm-offsets.h and asm-offsets.h are generated based > > > > > > > on U-Boot proper config options. The same asm-offsets headers are used > > > > > > > for building U-Boot SPL/TPL, which causes potential offset mismatch if > > > > > > > U-Boot proper has different config options from U-Boot SPL/TPL. > > > > > > > > > > > > > > This commit adds: > > > > > > > spl/include/generated/(generic-)asm-offsets.h > > > > > > > tpl/include/generated/(generic-)asm-offsets.h > > > > > > > > > > > > > > spl/include/generated/(generic-)asm-offsets.h is generated if > > > > > > > CONFIG_SPL=y, and included when building SPL. > > > > > > > > > > > > > > tpl/include/generated/(generic-)asm-offsets.h is generated if > > > > > > > CONFIG_TPL=y, and included when building TPL. > > > > > > > > > > > > > > They are created before Kbuild descends into SPL/TPL object directories > > > > > > > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c > > > > > > > includes a bunch of headers. > > > > > > > > > > > > > > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h > > > > > > > is searched in {spl,tpl}/include/generated/. > > > > > > > > > > > > > > Requested-by: Bin Meng <bmeng.cn at gmail.com> > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org> > > > > > > > --- > > > > > > > > > > > > > > Changes in v2: None > > > > > > > > > > > > > > Kbuild | 4 ++-- > > > > > > > scripts/Makefile.spl | 10 ++++++++-- > > > > > > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > > > > It works like a charm. Thanks! > > > > > > > > > > > > Tested-by: Bin Meng <bmeng.cn at gmail.com> > > > > > > > > > > It looks only patch 1 and 2 in this series were applied. This patch was missed. > > > > > > > > So, I was going to ask and forgot, sorry. Why do we need to have these > > > > differ between TPL/SPL/U-Boot itself, and then why is that good? I > > > > > > Because currently the offsets > > > {spl,tpl}/include/generated/(generic-)asm-offsets.h are generated per > > > the U-Boot config options. But SPL and TPL are not guaranteed to have > > > the same config options. > > > > > > For example it's possible to have SPL/TPL turned on > > > {SPL,TPL}_CONFIG_XXX but U-Boot to turn off CONFIG_XXX, or vice verse. > > > > > > To me this is more like a bug fix other than a feature. > > > > Ah, OK. But shouldn't we make sure that asm-offsets are consistent > > between SPL/TPL/U-Boot? Or is there a use-case where it makes sense for > > them to differ intentionally? > > I don't know other boards, but for now RISC-V does turn on SPL_SMP in > SPL, but does not turn on SMP in U-Boot S-mode. U-Boot M-mode can > still turn on SMP. OK. I'd really like to have it confirmed that it's a good idea to have that value differ between the modes and it's not a bug that should be fixed. Thanks!
On Wed, Apr 29, 2020 at 12:01 AM Bin Meng <bmeng.cn at gmail.com> wrote: > > Hi Tom, > > On Tue, Apr 28, 2020 at 10:46 PM Tom Rini <trini at konsulko.com> wrote: > > > > On Mon, Apr 27, 2020 at 10:50:48PM +0800, Bin Meng wrote: > > > Hi Tom, > > > > > > On Mon, Apr 27, 2020 at 9:37 PM Tom Rini <trini at konsulko.com> wrote: > > > > > > > > On Mon, Apr 27, 2020 at 09:37:28AM +0800, Bin Meng wrote: > > > > > Hi Tom, > > > > > > > > > > On Sat, Apr 18, 2020 at 8:37 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > > > > > > > > > On Fri, Apr 17, 2020 at 3:22 PM Masahiro Yamada <masahiroy at kernel.org> wrote: > > > > > > > > > > > > > > Currently generic-asm-offsets.h and asm-offsets.h are generated based > > > > > > > on U-Boot proper config options. The same asm-offsets headers are used > > > > > > > for building U-Boot SPL/TPL, which causes potential offset mismatch if > > > > > > > U-Boot proper has different config options from U-Boot SPL/TPL. > > > > > > > > > > > > > > This commit adds: > > > > > > > spl/include/generated/(generic-)asm-offsets.h > > > > > > > tpl/include/generated/(generic-)asm-offsets.h > > > > > > > > > > > > > > spl/include/generated/(generic-)asm-offsets.h is generated if > > > > > > > CONFIG_SPL=y, and included when building SPL. > > > > > > > > > > > > > > tpl/include/generated/(generic-)asm-offsets.h is generated if > > > > > > > CONFIG_TPL=y, and included when building TPL. > > > > > > > > > > > > > > They are created before Kbuild descends into SPL/TPL object directories > > > > > > > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c > > > > > > > includes a bunch of headers. > > > > > > > > > > > > > > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h > > > > > > > is searched in {spl,tpl}/include/generated/. > > > > > > > > > > > > > > Requested-by: Bin Meng <bmeng.cn at gmail.com> > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org> > > > > > > > --- > > > > > > > > > > > > > > Changes in v2: None > > > > > > > > > > > > > > Kbuild | 4 ++-- > > > > > > > scripts/Makefile.spl | 10 ++++++++-- > > > > > > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > > > > It works like a charm. Thanks! > > > > > > > > > > > > Tested-by: Bin Meng <bmeng.cn at gmail.com> > > > > > > > > > > It looks only patch 1 and 2 in this series were applied. This patch was missed. > > > > > > > > So, I was going to ask and forgot, sorry. Why do we need to have these > > > > differ between TPL/SPL/U-Boot itself, and then why is that good? I > > > > > > Because currently the offsets > > > {spl,tpl}/include/generated/(generic-)asm-offsets.h are generated per > > > the U-Boot config options. But SPL and TPL are not guaranteed to have > > > the same config options. > > > > > > For example it's possible to have SPL/TPL turned on > > > {SPL,TPL}_CONFIG_XXX but U-Boot to turn off CONFIG_XXX, or vice verse. > > > > > > To me this is more like a bug fix other than a feature. > > > > Ah, OK. But shouldn't we make sure that asm-offsets are consistent > > between SPL/TPL/U-Boot? Or is there a use-case where it makes sense for > > them to differ intentionally? Maybe, save memory for SPL/TPL ? We can say "do not do that", but it is fragile. People are adding #ifdef around to all sort of structs, and it is difficult to guess which ones are used to construct asm-offsets.
On Wed, Apr 29, 2020 at 12:20:46AM +0900, Masahiro Yamada wrote: > On Wed, Apr 29, 2020 at 12:01 AM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > Hi Tom, > > > > On Tue, Apr 28, 2020 at 10:46 PM Tom Rini <trini at konsulko.com> wrote: > > > > > > On Mon, Apr 27, 2020 at 10:50:48PM +0800, Bin Meng wrote: > > > > Hi Tom, > > > > > > > > On Mon, Apr 27, 2020 at 9:37 PM Tom Rini <trini at konsulko.com> wrote: > > > > > > > > > > On Mon, Apr 27, 2020 at 09:37:28AM +0800, Bin Meng wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Sat, Apr 18, 2020 at 8:37 PM Bin Meng <bmeng.cn at gmail.com> wrote: > > > > > > > > > > > > > > On Fri, Apr 17, 2020 at 3:22 PM Masahiro Yamada <masahiroy at kernel.org> wrote: > > > > > > > > > > > > > > > > Currently generic-asm-offsets.h and asm-offsets.h are generated based > > > > > > > > on U-Boot proper config options. The same asm-offsets headers are used > > > > > > > > for building U-Boot SPL/TPL, which causes potential offset mismatch if > > > > > > > > U-Boot proper has different config options from U-Boot SPL/TPL. > > > > > > > > > > > > > > > > This commit adds: > > > > > > > > spl/include/generated/(generic-)asm-offsets.h > > > > > > > > tpl/include/generated/(generic-)asm-offsets.h > > > > > > > > > > > > > > > > spl/include/generated/(generic-)asm-offsets.h is generated if > > > > > > > > CONFIG_SPL=y, and included when building SPL. > > > > > > > > > > > > > > > > tpl/include/generated/(generic-)asm-offsets.h is generated if > > > > > > > > CONFIG_TPL=y, and included when building TPL. > > > > > > > > > > > > > > > > They are created before Kbuild descends into SPL/TPL object directories > > > > > > > > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c > > > > > > > > includes a bunch of headers. > > > > > > > > > > > > > > > > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h > > > > > > > > is searched in {spl,tpl}/include/generated/. > > > > > > > > > > > > > > > > Requested-by: Bin Meng <bmeng.cn at gmail.com> > > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org> > > > > > > > > --- > > > > > > > > > > > > > > > > Changes in v2: None > > > > > > > > > > > > > > > > Kbuild | 4 ++-- > > > > > > > > scripts/Makefile.spl | 10 ++++++++-- > > > > > > > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > > > > > > > > It works like a charm. Thanks! > > > > > > > > > > > > > > Tested-by: Bin Meng <bmeng.cn at gmail.com> > > > > > > > > > > > > It looks only patch 1 and 2 in this series were applied. This patch was missed. > > > > > > > > > > So, I was going to ask and forgot, sorry. Why do we need to have these > > > > > differ between TPL/SPL/U-Boot itself, and then why is that good? I > > > > > > > > Because currently the offsets > > > > {spl,tpl}/include/generated/(generic-)asm-offsets.h are generated per > > > > the U-Boot config options. But SPL and TPL are not guaranteed to have > > > > the same config options. > > > > > > > > For example it's possible to have SPL/TPL turned on > > > > {SPL,TPL}_CONFIG_XXX but U-Boot to turn off CONFIG_XXX, or vice verse. > > > > > > > > To me this is more like a bug fix other than a feature. > > > > > > Ah, OK. But shouldn't we make sure that asm-offsets are consistent > > > between SPL/TPL/U-Boot? Or is there a use-case where it makes sense for > > > them to differ intentionally? > > Maybe, save memory for SPL/TPL ? > We can say "do not do that", but it is fragile. > > People are adding #ifdef around to all sort of structs, > and it is difficult to guess which ones are used to > construct asm-offsets. So, you're right. I'm doing a build now to check the differences, but fairly often SPL and TPL disagree here, related to malloc and similar it looks like. So this is a bug-fix and I'll apply it shortly. Thanks!
On Fri, Apr 17, 2020 at 04:21:37PM +0900, Masahiro Yamada wrote: > Currently generic-asm-offsets.h and asm-offsets.h are generated based > on U-Boot proper config options. The same asm-offsets headers are used > for building U-Boot SPL/TPL, which causes potential offset mismatch if > U-Boot proper has different config options from U-Boot SPL/TPL. > > This commit adds: > spl/include/generated/(generic-)asm-offsets.h > tpl/include/generated/(generic-)asm-offsets.h > > spl/include/generated/(generic-)asm-offsets.h is generated if > CONFIG_SPL=y, and included when building SPL. > > tpl/include/generated/(generic-)asm-offsets.h is generated if > CONFIG_TPL=y, and included when building TPL. > > They are created before Kbuild descends into SPL/TPL object directories > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c > includes a bunch of headers. > > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h > is searched in {spl,tpl}/include/generated/. > > Requested-by: Bin Meng <bmeng.cn at gmail.com> > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org> > Tested-by: Bin Meng <bmeng.cn at gmail.com> Applied to u-boot/master, thanks!
diff --git a/Kbuild b/Kbuild index 6014f7c34a..1eac091594 100644 --- a/Kbuild +++ b/Kbuild @@ -10,7 +10,7 @@ generic-offsets-file := include/generated/generic-asm-offsets.h always := $(generic-offsets-file) targets := lib/asm-offsets.s -$(obj)/$(generic-offsets-file): lib/asm-offsets.s FORCE +$(obj)/$(generic-offsets-file): $(obj)/lib/asm-offsets.s FORCE $(call filechk,offsets,__GENERIC_ASM_OFFSETS_H__) ##### @@ -25,5 +25,5 @@ targets += arch/$(ARCH)/lib/asm-offsets.s CFLAGS_asm-offsets.o := -DDO_DEPS_ONLY -$(obj)/$(offsets-file): arch/$(ARCH)/lib/asm-offsets.s FORCE +$(obj)/$(offsets-file): $(obj)/arch/$(ARCH)/lib/asm-offsets.s FORCE $(call filechk,offsets,__ASM_OFFSETS_H__) diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 4c2c0567c5..6741ef911e 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -22,6 +22,8 @@ include $(srctree)/scripts/Kbuild.include -include include/config/auto.conf -include $(obj)/include/autoconf.mk +UBOOTINCLUDE := -I$(obj)/include $(UBOOTINCLUDE) + KBUILD_CPPFLAGS += -DCONFIG_SPL_BUILD ifeq ($(CONFIG_TPL_BUILD),y) KBUILD_CPPFLAGS += -DCONFIG_TPL_BUILD @@ -311,7 +313,7 @@ cmd_plat = $(CC) $(c_flags) -c $< -o $(filter-out $(PHONY),$@) targets += $(obj)/dts/dt-platdata.o $(obj)/dts/dt-platdata.o: $(obj)/dts/dt-platdata.c \ - include/generated/dt-structs-gen.h FORCE + include/generated/dt-structs-gen.h prepare FORCE $(call if_changed,plat) PHONY += dts_dir @@ -422,9 +424,13 @@ $(obj)/$(SPL_BIN): $(u-boot-spl-platdata) $(u-boot-spl-init) \ $(sort $(u-boot-spl-init) $(u-boot-spl-main)): $(u-boot-spl-dirs) ; PHONY += $(u-boot-spl-dirs) -$(u-boot-spl-dirs): $(u-boot-spl-platdata) +$(u-boot-spl-dirs): $(u-boot-spl-platdata) prepare $(Q)$(MAKE) $(build)=$@ +PHONY += prepare +prepare: + $(Q)$(MAKE) $(build)=$(obj)/. + quiet_cmd_cpp_lds = LDS $@ cmd_cpp_lds = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \ -D__ASSEMBLY__ -x assembler-with-cpp -std=c99 -P -o $@ $<
Currently generic-asm-offsets.h and asm-offsets.h are generated based on U-Boot proper config options. The same asm-offsets headers are used for building U-Boot SPL/TPL, which causes potential offset mismatch if U-Boot proper has different config options from U-Boot SPL/TPL. This commit adds: spl/include/generated/(generic-)asm-offsets.h tpl/include/generated/(generic-)asm-offsets.h spl/include/generated/(generic-)asm-offsets.h is generated if CONFIG_SPL=y, and included when building SPL. tpl/include/generated/(generic-)asm-offsets.h is generated if CONFIG_TPL=y, and included when building TPL. They are created before Kbuild descends into SPL/TPL object directories and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c includes a bunch of headers. Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h is searched in {spl,tpl}/include/generated/. Requested-by: Bin Meng <bmeng.cn at gmail.com> Signed-off-by: Masahiro Yamada <masahiroy at kernel.org> --- Changes in v2: None Kbuild | 4 ++-- scripts/Makefile.spl | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-)