* [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.