| Message ID | 20230629053130.3976-2-jeanmichel.hautbois@yoseli.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for the MCF54418 Coldfire | expand |
Jean-Michel, All, On 2023-06-29 07:31 +0200, Jean-Michel Hautbois spake thusly: > The m68k family ony has one Coldfire variant, namely the 5208. Add the > support for the MCF54418 CPU in the configuration file. As your cover-letter said, eda11417beaf (m68k: remove BR2_GCC_TARGET_ARCH), setting it breaks use of external toolchains. Granted, the commit log was not very expansive in why it did break them which required dropping setting m68k/cf. However, it did state that the m68k/cf defconfigs where working perfectly well without it. So, your commit log should explain how/why it is safe to re-introduce BR2_GCC_TARGET_ARCH. I see you only set ig for this new CPU variant, so maybe that's enough of a reason. Still, it'd be nice to have it explained in plain letters. Also, it'd be nice if you could try with an external toolchain, and explain that in the commit log. If you do not have an external toolchain available, you can generate one with Buildroot, and re-use it as an external toolchain. Regards, Yann E. MORIN. > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org> > --- > arch/Config.in.m68k | 9 +++++++++ > toolchain/Config.in | 1 + > 2 files changed, 10 insertions(+) > > diff --git a/arch/Config.in.m68k b/arch/Config.in.m68k > index 9fd22aaf1e..6aa409b272 100644 > --- a/arch/Config.in.m68k > +++ b/arch/Config.in.m68k > @@ -33,11 +33,20 @@ config BR2_m68k_cf5208 > select BR2_m68k_cf > select BR2_SOFT_FLOAT > > +config BR2_m68k_cf54418 > + bool "54418" > + select BR2_m68k_cf > + select BR2_USE_MMU > + select BR2_SOFT_FLOAT > endchoice > > config BR2_GCC_TARGET_CPU > default "68040" if BR2_m68k_68040 > default "5208" if BR2_m68k_cf5208 > + default "54455" if BR2_m68k_cf54418 > + > +config BR2_GCC_TARGET_ARCH > + default "cf" if BR2_m68k_cf54418 > > config BR2_READELF_ARCH_NAME > default "MC68000" > diff --git a/toolchain/Config.in b/toolchain/Config.in > index ff0eb93017..ec2185cb39 100644 > --- a/toolchain/Config.in > +++ b/toolchain/Config.in > @@ -95,6 +95,7 @@ config BR2_TOOLCHAIN_SUPPORTS_ALWAYS_LOCKFREE_ATOMIC_INTS > depends on !BR2_ARM_CPU_ARMV5 > depends on !BR2_sparc_v8 > depends on !BR2_m68k_cf5208 > + depends on !BR2_m68k_cf54418 > > # GCC uses thunk functions to adjust the 'this' pointer when calling > # C++ member functions in classes derived with multiple inheritance. > -- > 2.39.2 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Jean-Michel, All, Second round... For just a minor detail this time. ;-) On 2023-06-29 07:31 +0200, Jean-Michel Hautbois spake thusly: > The m68k family ony has one Coldfire variant, namely the 5208. Add the > support for the MCF54418 CPU in the configuration file. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org> > --- > arch/Config.in.m68k | 9 +++++++++ > toolchain/Config.in | 1 + > 2 files changed, 10 insertions(+) > > diff --git a/arch/Config.in.m68k b/arch/Config.in.m68k > index 9fd22aaf1e..6aa409b272 100644 > --- a/arch/Config.in.m68k > +++ b/arch/Config.in.m68k > @@ -33,11 +33,20 @@ config BR2_m68k_cf5208 > select BR2_m68k_cf > select BR2_SOFT_FLOAT > > +config BR2_m68k_cf54418 > + bool "54418" > + select BR2_m68k_cf > + select BR2_USE_MMU > + select BR2_SOFT_FLOAT You missed adding an empty line between the new entry and the 'endchoice' statement, like there was before. Regards, Yann E. MORIN. > endchoice > > config BR2_GCC_TARGET_CPU > default "68040" if BR2_m68k_68040 > default "5208" if BR2_m68k_cf5208 > + default "54455" if BR2_m68k_cf54418 > + > +config BR2_GCC_TARGET_ARCH > + default "cf" if BR2_m68k_cf54418 > > config BR2_READELF_ARCH_NAME > default "MC68000" > diff --git a/toolchain/Config.in b/toolchain/Config.in > index ff0eb93017..ec2185cb39 100644 > --- a/toolchain/Config.in > +++ b/toolchain/Config.in > @@ -95,6 +95,7 @@ config BR2_TOOLCHAIN_SUPPORTS_ALWAYS_LOCKFREE_ATOMIC_INTS > depends on !BR2_ARM_CPU_ARMV5 > depends on !BR2_sparc_v8 > depends on !BR2_m68k_cf5208 > + depends on !BR2_m68k_cf54418 > > # GCC uses thunk functions to adjust the 'this' pointer when calling > # C++ member functions in classes derived with multiple inheritance. > -- > 2.39.2 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Hi Yann, On 03/07/2023 20:55, Yann E. MORIN wrote: > Jean-Michel, All, > > On 2023-06-29 07:31 +0200, Jean-Michel Hautbois spake thusly: >> The m68k family ony has one Coldfire variant, namely the 5208. Add the >> support for the MCF54418 CPU in the configuration file. > > As your cover-letter said, eda11417beaf (m68k: remove > BR2_GCC_TARGET_ARCH), setting it breaks use of external toolchains. > > Granted, the commit log was not very expansive in why it did break them > which required dropping setting m68k/cf. Indeed. Well, I did not found the root cause, but I can say for sure that the USE_MMU configuration makes it impossible to build the buildroot toolchain. > However, it did state that the m68k/cf defconfigs where working > perfectly well without it. > > So, your commit log should explain how/why it is safe to re-introduce > BR2_GCC_TARGET_ARCH. I see you only set ig for this new CPU variant, so > maybe that's enough of a reason. > > Still, it'd be nice to have it explained in plain letters. OK, so get the content from the plain letter and have it in this commit message too ;-). > Also, it'd be nice if you could try with an external toolchain, and > explain that in the commit log. If you do not have an external toolchain > available, you can generate one with Buildroot, and re-use it as an > external toolchain. Sadly, the only external toolchain I tried is the one from bootlin, and it is not compatible with a CPU which has a MMU. Considering I might miss some things ! JM > > Regards, > Yann E. MORIN. > >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org> >> --- >> arch/Config.in.m68k | 9 +++++++++ >> toolchain/Config.in | 1 + >> 2 files changed, 10 insertions(+) >> >> diff --git a/arch/Config.in.m68k b/arch/Config.in.m68k >> index 9fd22aaf1e..6aa409b272 100644 >> --- a/arch/Config.in.m68k >> +++ b/arch/Config.in.m68k >> @@ -33,11 +33,20 @@ config BR2_m68k_cf5208 >> select BR2_m68k_cf >> select BR2_SOFT_FLOAT >> >> +config BR2_m68k_cf54418 >> + bool "54418" >> + select BR2_m68k_cf >> + select BR2_USE_MMU >> + select BR2_SOFT_FLOAT >> endchoice >> >> config BR2_GCC_TARGET_CPU >> default "68040" if BR2_m68k_68040 >> default "5208" if BR2_m68k_cf5208 >> + default "54455" if BR2_m68k_cf54418 >> + >> +config BR2_GCC_TARGET_ARCH >> + default "cf" if BR2_m68k_cf54418 >> >> config BR2_READELF_ARCH_NAME >> default "MC68000" >> diff --git a/toolchain/Config.in b/toolchain/Config.in >> index ff0eb93017..ec2185cb39 100644 >> --- a/toolchain/Config.in >> +++ b/toolchain/Config.in >> @@ -95,6 +95,7 @@ config BR2_TOOLCHAIN_SUPPORTS_ALWAYS_LOCKFREE_ATOMIC_INTS >> depends on !BR2_ARM_CPU_ARMV5 >> depends on !BR2_sparc_v8 >> depends on !BR2_m68k_cf5208 >> + depends on !BR2_m68k_cf54418 >> >> # GCC uses thunk functions to adjust the 'this' pointer when calling >> # C++ member functions in classes derived with multiple inheritance. >> -- >> 2.39.2 >> >> _______________________________________________ >> buildroot mailing list >> buildroot@buildroot.org >> https://lists.buildroot.org/mailman/listinfo/buildroot >
Jean-Michel, All, On 2023-07-03 22:37 +0200, Jean-Michel Hautbois spake thusly: > On 03/07/2023 20:55, Yann E. MORIN wrote: > >On 2023-06-29 07:31 +0200, Jean-Michel Hautbois spake thusly: > >>The m68k family ony has one Coldfire variant, namely the 5208. Add the > >>support for the MCF54418 CPU in the configuration file. > >As your cover-letter said, eda11417beaf (m68k: remove > >BR2_GCC_TARGET_ARCH), setting it breaks use of external toolchains. [--SNIP--] > >So, your commit log should explain how/why it is safe to re-introduce > >BR2_GCC_TARGET_ARCH. I see you only set ig for this new CPU variant, so > >maybe that's enough of a reason. > > > >Still, it'd be nice to have it explained in plain letters. > > OK, so get the content from the plain letter and have it in this commit > message too ;-). Yes, the reference to commit eda11417beaf is very important to have in the nez commit mess is very important to have in the nez commit messagee > >Also, it'd be nice if you could try with an external toolchain, and > >explain that in the commit log. If you do not have an external toolchain > >available, you can generate one with Buildroot, and re-use it as an > >external toolchain. > Sadly, the only external toolchain I tried is the one from bootlin, and it > is not compatible with a CPU which has a MMU. Considering I might miss some > things ! If there is no other pre-built toolchain, then build one with Buildroot (with this change!), and re-user it as an external toolchain, just to be sure that this case at least still works; see the manual [0] on how to do that. [0] https://buildroot.org/downloads/manual/manual.html#_advanced_usage Regards, Yann E. MORIN.
diff --git a/arch/Config.in.m68k b/arch/Config.in.m68k index 9fd22aaf1e..6aa409b272 100644 --- a/arch/Config.in.m68k +++ b/arch/Config.in.m68k @@ -33,11 +33,20 @@ config BR2_m68k_cf5208 select BR2_m68k_cf select BR2_SOFT_FLOAT +config BR2_m68k_cf54418 + bool "54418" + select BR2_m68k_cf + select BR2_USE_MMU + select BR2_SOFT_FLOAT endchoice config BR2_GCC_TARGET_CPU default "68040" if BR2_m68k_68040 default "5208" if BR2_m68k_cf5208 + default "54455" if BR2_m68k_cf54418 + +config BR2_GCC_TARGET_ARCH + default "cf" if BR2_m68k_cf54418 config BR2_READELF_ARCH_NAME default "MC68000" diff --git a/toolchain/Config.in b/toolchain/Config.in index ff0eb93017..ec2185cb39 100644 --- a/toolchain/Config.in +++ b/toolchain/Config.in @@ -95,6 +95,7 @@ config BR2_TOOLCHAIN_SUPPORTS_ALWAYS_LOCKFREE_ATOMIC_INTS depends on !BR2_ARM_CPU_ARMV5 depends on !BR2_sparc_v8 depends on !BR2_m68k_cf5208 + depends on !BR2_m68k_cf54418 # GCC uses thunk functions to adjust the 'this' pointer when calling # C++ member functions in classes derived with multiple inheritance.
The m68k family ony has one Coldfire variant, namely the 5208. Add the support for the MCF54418 CPU in the configuration file. Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org> --- arch/Config.in.m68k | 9 +++++++++ toolchain/Config.in | 1 + 2 files changed, 10 insertions(+)