* [PATCH] MIPS: OCTEON: Fix the boot broken when using built-in DTB @ 2021-01-09 11:49 Kevin Hao 2021-01-18 18:24 ` Thomas Bogendoerfer 0 siblings, 1 reply; 4+ messages in thread From: Kevin Hao @ 2021-01-09 11:49 UTC (permalink / raw) To: Thomas Bogendoerfer; +Cc: linux-mips For the OCTEON boards, it need to patch the built-in DTB before using it. Previously it judges if it is a built-in DTB by checking fw_passed_dtb. But after commit 37e5c69ffd41 ("MIPS: head.S: Init fw_passed_dtb to builtin DTB", the fw_passed_dtb is initialized even when using built-in DTB. This causes the OCTEON boards boot broken due to an unpatched built-in DTB is used. Add more checks to judge if we really use built-in DTB or not. Fixed: 37e5c69ffd41 ("MIPS: head.S: Init fw_passed_dtb to builtin DTB") Cc: stable@vger.kernel.org Signed-off-by: Kevin Hao <haokexin@gmail.com> --- arch/mips/cavium-octeon/setup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c index 982826ba0ef7..41d9c80e9666 100644 --- a/arch/mips/cavium-octeon/setup.c +++ b/arch/mips/cavium-octeon/setup.c @@ -1149,7 +1149,8 @@ void __init device_tree_init(void) bool do_prune; bool fill_mac; - if (fw_passed_dtb) { + if (fw_passed_dtb && (fw_passed_dtb != (ulong)&__dtb_octeon_68xx_begin) && + (fw_passed_dtb != (ulong)&__dtb_octeon_3xxx_begin)) { fdt = (void *)fw_passed_dtb; do_prune = false; fill_mac = true; -- 2.29.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS: OCTEON: Fix the boot broken when using built-in DTB 2021-01-09 11:49 [PATCH] MIPS: OCTEON: Fix the boot broken when using built-in DTB Kevin Hao @ 2021-01-18 18:24 ` Thomas Bogendoerfer 2021-01-19 10:56 ` Kevin Hao 0 siblings, 1 reply; 4+ messages in thread From: Thomas Bogendoerfer @ 2021-01-18 18:24 UTC (permalink / raw) To: Kevin Hao; +Cc: linux-mips On Sat, Jan 09, 2021 at 07:49:58PM +0800, Kevin Hao wrote: > For the OCTEON boards, it need to patch the built-in DTB before using > it. Previously it judges if it is a built-in DTB by checking > fw_passed_dtb. But after commit 37e5c69ffd41 ("MIPS: head.S: Init > fw_passed_dtb to builtin DTB", the fw_passed_dtb is initialized even > when using built-in DTB. This causes the OCTEON boards boot broken due > to an unpatched built-in DTB is used. Add more checks to judge if we > really use built-in DTB or not. > > Fixed: 37e5c69ffd41 ("MIPS: head.S: Init fw_passed_dtb to builtin DTB") > Cc: stable@vger.kernel.org > Signed-off-by: Kevin Hao <haokexin@gmail.com> > --- > arch/mips/cavium-octeon/setup.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c > index 982826ba0ef7..41d9c80e9666 100644 > --- a/arch/mips/cavium-octeon/setup.c > +++ b/arch/mips/cavium-octeon/setup.c > @@ -1149,7 +1149,8 @@ void __init device_tree_init(void) > bool do_prune; > bool fill_mac; > > - if (fw_passed_dtb) { > + if (fw_passed_dtb && (fw_passed_dtb != (ulong)&__dtb_octeon_68xx_begin) && > + (fw_passed_dtb != (ulong)&__dtb_octeon_3xxx_begin)) { well, if someone add a third dtb do the builtin DTBs, this might fail again. Let's fix it for real... > fdt = (void *)fw_passed_dtb; > do_prune = false; > fill_mac = true; .... IMHO the real bug is d9df9fb901d2 MIPS: Octeon: Remove special handling of CONFIG_MIPS_ELF_APPENDED_DTB=y. I'm tending to simply revert that commit. A different option would be to not place the two __dtb_octeon dtbs into the builtin dtb section, which looks like more work for not much gain... Thomas. -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea. [ RFC1925, 2.3 ] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS: OCTEON: Fix the boot broken when using built-in DTB 2021-01-18 18:24 ` Thomas Bogendoerfer @ 2021-01-19 10:56 ` Kevin Hao 2021-01-20 17:39 ` Thomas Bogendoerfer 0 siblings, 1 reply; 4+ messages in thread From: Kevin Hao @ 2021-01-19 10:56 UTC (permalink / raw) To: Thomas Bogendoerfer; +Cc: linux-mips [-- Attachment #1: Type: text/plain, Size: 2360 bytes --] On Mon, Jan 18, 2021 at 07:24:19PM +0100, Thomas Bogendoerfer wrote: > On Sat, Jan 09, 2021 at 07:49:58PM +0800, Kevin Hao wrote: > > For the OCTEON boards, it need to patch the built-in DTB before using > > it. Previously it judges if it is a built-in DTB by checking > > fw_passed_dtb. But after commit 37e5c69ffd41 ("MIPS: head.S: Init > > fw_passed_dtb to builtin DTB", the fw_passed_dtb is initialized even > > when using built-in DTB. This causes the OCTEON boards boot broken due > > to an unpatched built-in DTB is used. Add more checks to judge if we > > really use built-in DTB or not. > > > > Fixed: 37e5c69ffd41 ("MIPS: head.S: Init fw_passed_dtb to builtin DTB") > > Cc: stable@vger.kernel.org > > Signed-off-by: Kevin Hao <haokexin@gmail.com> > > --- > > arch/mips/cavium-octeon/setup.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c > > index 982826ba0ef7..41d9c80e9666 100644 > > --- a/arch/mips/cavium-octeon/setup.c > > +++ b/arch/mips/cavium-octeon/setup.c > > @@ -1149,7 +1149,8 @@ void __init device_tree_init(void) > > bool do_prune; > > bool fill_mac; > > > > - if (fw_passed_dtb) { > > + if (fw_passed_dtb && (fw_passed_dtb != (ulong)&__dtb_octeon_68xx_begin) && > > + (fw_passed_dtb != (ulong)&__dtb_octeon_3xxx_begin)) { > > well, if someone add a third dtb do the builtin DTBs, this might fail > again. Let's fix it for real... > > > fdt = (void *)fw_passed_dtb; > > do_prune = false; > > fill_mac = true; > > .... IMHO the real bug is d9df9fb901d2 MIPS: Octeon: Remove special handling > of CONFIG_MIPS_ELF_APPENDED_DTB=y. I'm tending to simply revert that commit. Yes, this indeed seem much better. I will send a patch to revert d9df9fb901d2. Another issue is that the name of fw_passed_dtb seems pretty confusion after the change of commit 37e5c69ffd41. Could we rename it to something like final_dtb_addr? Thanks, Kevin > A different option would be to not place the two __dtb_octeon dtbs into the > builtin dtb section, which looks like more work for not much gain... > > Thomas. > > -- > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a > good idea. [ RFC1925, 2.3 ] [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS: OCTEON: Fix the boot broken when using built-in DTB 2021-01-19 10:56 ` Kevin Hao @ 2021-01-20 17:39 ` Thomas Bogendoerfer 0 siblings, 0 replies; 4+ messages in thread From: Thomas Bogendoerfer @ 2021-01-20 17:39 UTC (permalink / raw) To: Kevin Hao; +Cc: linux-mips On Tue, Jan 19, 2021 at 06:56:48PM +0800, Kevin Hao wrote: > > .... IMHO the real bug is d9df9fb901d2 MIPS: Octeon: Remove special handling > > of CONFIG_MIPS_ELF_APPENDED_DTB=y. I'm tending to simply revert that commit. > > Yes, this indeed seem much better. I will send a patch to revert d9df9fb901d2. > Another issue is that the name of fw_passed_dtb seems pretty confusion after the > change of commit 37e5c69ffd41. Could we rename it to something like final_dtb_addr? this wouldn't make the mess smaller, IMHO. My idea is to add a helper function, which deals with all the possible sources of dtbs, which could be used instead of fw_passed_dtb. This gets rid of the ugly #ifdefry in head.S and hopefull makes future changes less error prone. Thomas. -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea. [ RFC1925, 2.3 ] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-20 22:16 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-09 11:49 [PATCH] MIPS: OCTEON: Fix the boot broken when using built-in DTB Kevin Hao 2021-01-18 18:24 ` Thomas Bogendoerfer 2021-01-19 10:56 ` Kevin Hao 2021-01-20 17:39 ` Thomas Bogendoerfer
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.