of/fdt: Remove PPC32 longtrail hack in memory scan

Message ID 20180727053555.26596-1-mpe@ellerman.id.au
State New
Headers show
Series
  • of/fdt: Remove PPC32 longtrail hack in memory scan
Related show

Checks

Context Check Description
snowpatch_ozlabs/build-ppc32 success Test build-ppc32 on branch next
snowpatch_ozlabs/build-ppc64e success Test build-ppc64e on branch next
snowpatch_ozlabs/build-ppc64be success Test build-ppc64be on branch next
snowpatch_ozlabs/build-ppc64le success Test build-ppc64le on branch next
snowpatch_ozlabs/checkpatch fail Test checkpatch on branch next
snowpatch_ozlabs/apply_patch success next/apply_patch Successfully applied

Commit Message

Michael Ellerman July 27, 2018, 5:35 a.m.
When the OF code was originally made common by Grant in commit
51975db0b733 ("of/flattree: merge early_init_dt_scan_memory() common
code") (Feb 2010), the common code inherited a hack to handle
PPC "longtrail" machines, which had a "memory@0" node with no
device_type.

That check was then made to only apply to PPC32 in b44aa25d20e2 ("of:
Handle memory@0 node on PPC32 only") (May 2014).

But according to Paul Mackerras the "longtrail" machines are long
dead, if they were ever seen in the wild at all. If someone does still
have one, we can handle this firmware wart in powerpc platform code.

So remove the hack once and for all.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 drivers/of/fdt.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

Comments

Rob Herring July 27, 2018, 5:12 p.m. | #1
On Thu, Jul 26, 2018 at 11:36 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> When the OF code was originally made common by Grant in commit
> 51975db0b733 ("of/flattree: merge early_init_dt_scan_memory() common
> code") (Feb 2010), the common code inherited a hack to handle
> PPC "longtrail" machines, which had a "memory@0" node with no
> device_type.
>
> That check was then made to only apply to PPC32 in b44aa25d20e2 ("of:
> Handle memory@0 node on PPC32 only") (May 2014).
>
> But according to Paul Mackerras the "longtrail" machines are long
> dead, if they were ever seen in the wild at all. If someone does still
> have one, we can handle this firmware wart in powerpc platform code.
>
> So remove the hack once and for all.

Yay. I guess Power Macs and other quirks will never die...

I'll queue this up.

Rob
Michael Ellerman July 30, 2018, 10:47 a.m. | #2
Rob Herring <robh+dt@kernel.org> writes:
> On Thu, Jul 26, 2018 at 11:36 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> When the OF code was originally made common by Grant in commit
>> 51975db0b733 ("of/flattree: merge early_init_dt_scan_memory() common
>> code") (Feb 2010), the common code inherited a hack to handle
>> PPC "longtrail" machines, which had a "memory@0" node with no
>> device_type.
>>
>> That check was then made to only apply to PPC32 in b44aa25d20e2 ("of:
>> Handle memory@0 node on PPC32 only") (May 2014).
>>
>> But according to Paul Mackerras the "longtrail" machines are long
>> dead, if they were ever seen in the wild at all. If someone does still
>> have one, we can handle this firmware wart in powerpc platform code.
>>
>> So remove the hack once and for all.
>
> Yay. I guess Power Macs and other quirks will never die...

Not soon.

In base.c I see:
 - the hack in arch_find_n_match_cpu_physical_id()
   - we should just move that into arch code, it's a __weak arch hook
     after all.
 - a PPC hack in of_alias_scan(), I guess we need to retain that
   behaviour, but it's pretty minor anyway.

In address.c there's the powermac empty ranges hack. Seems like we could
fix that just by creating empty `ranges` properties in fixup_device_tree().
I don't think we support booting powermacs other than via prom_init(). (Ben?)

> I'll queue this up.

cheers
Rob Herring July 30, 2018, 5:19 p.m. | #3
On Mon, Jul 30, 2018 at 4:47 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Rob Herring <robh+dt@kernel.org> writes:
> > On Thu, Jul 26, 2018 at 11:36 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
> >> When the OF code was originally made common by Grant in commit
> >> 51975db0b733 ("of/flattree: merge early_init_dt_scan_memory() common
> >> code") (Feb 2010), the common code inherited a hack to handle
> >> PPC "longtrail" machines, which had a "memory@0" node with no
> >> device_type.
> >>
> >> That check was then made to only apply to PPC32 in b44aa25d20e2 ("of:
> >> Handle memory@0 node on PPC32 only") (May 2014).
> >>
> >> But according to Paul Mackerras the "longtrail" machines are long
> >> dead, if they were ever seen in the wild at all. If someone does still
> >> have one, we can handle this firmware wart in powerpc platform code.
> >>
> >> So remove the hack once and for all.
> >
> > Yay. I guess Power Macs and other quirks will never die...
>
> Not soon.
>
> In base.c I see:
>  - the hack in arch_find_n_match_cpu_physical_id()
>    - we should just move that into arch code, it's a __weak arch hook
>      after all.

Except then we'd have to export __of_find_n_match_cpu_property. I
somewhat prefer it like it is because arch specific functions tend to
encourage duplication. But if the implementation is completely
different like sparc, then yes, a separate implementation makes sense.

>  - a PPC hack in of_alias_scan(), I guess we need to retain that
>    behaviour, but it's pretty minor anyway.

It would be nice to know what platform(s) needs this as I don't have a
clue. It would also be nice if I had some dumps of DTs for some of
these OpenFirmware systems.

Rob
Michael Ellerman Aug. 7, 2018, 12:34 p.m. | #4
Rob Herring <robh+dt@kernel.org> writes:
> On Mon, Jul 30, 2018 at 4:47 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Rob Herring <robh+dt@kernel.org> writes:
>> > On Thu, Jul 26, 2018 at 11:36 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> >> When the OF code was originally made common by Grant in commit
>> >> 51975db0b733 ("of/flattree: merge early_init_dt_scan_memory() common
>> >> code") (Feb 2010), the common code inherited a hack to handle
>> >> PPC "longtrail" machines, which had a "memory@0" node with no
>> >> device_type.
>> >>
>> >> That check was then made to only apply to PPC32 in b44aa25d20e2 ("of:
>> >> Handle memory@0 node on PPC32 only") (May 2014).
>> >>
>> >> But according to Paul Mackerras the "longtrail" machines are long
>> >> dead, if they were ever seen in the wild at all. If someone does still
>> >> have one, we can handle this firmware wart in powerpc platform code.
>> >>
>> >> So remove the hack once and for all.
>> >
>> > Yay. I guess Power Macs and other quirks will never die...
>>
>> Not soon.
>>
>> In base.c I see:
>>  - the hack in arch_find_n_match_cpu_physical_id()
>>    - we should just move that into arch code, it's a __weak arch hook
>>      after all.
>
> Except then we'd have to export __of_find_n_match_cpu_property. I
> somewhat prefer it like it is because arch specific functions tend to
> encourage duplication. But if the implementation is completely
> different like sparc, then yes, a separate implementation makes sense.

OK I'll leave it as-is then.

>>  - a PPC hack in of_alias_scan(), I guess we need to retain that
>>    behaviour, but it's pretty minor anyway.
>
> It would be nice to know what platform(s) needs this as I don't have a
> clue.

Yeah.

> It would also be nice if I had some dumps of DTs for some of
> these OpenFirmware systems.

Yeah it would. What if I threw some in a git tree somewhere?

I guess fully unpacked is the most useful form, ie. just a direct copy
from /proc/device-tree.

cheers
Geert Uytterhoeven Aug. 8, 2018, 9:29 a.m. | #5
Hi Michael,

On Fri, Jul 27, 2018 at 7:36 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
> When the OF code was originally made common by Grant in commit
> 51975db0b733 ("of/flattree: merge early_init_dt_scan_memory() common
> code") (Feb 2010), the common code inherited a hack to handle
> PPC "longtrail" machines, which had a "memory@0" node with no
> device_type.
>
> That check was then made to only apply to PPC32 in b44aa25d20e2 ("of:
> Handle memory@0 node on PPC32 only") (May 2014).
>
> But according to Paul Mackerras the "longtrail" machines are long
> dead, if they were ever seen in the wild at all. If someone does still
> have one, we can handle this firmware wart in powerpc platform code.
>
> So remove the hack once and for all.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Thanks for your patch!

My LongTrail died in 2004.  I haven't heard since even longer about active
use from any of the other people I know that had one.

So:

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

However, recently Dominik (CC) send me an enquiry about it, so perhaps
he is a happy user?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Dominik Klein Aug. 8, 2018, 11:09 a.m. | #6
Hi guys,

On 08.08.2018 11:29, Geert Uytterhoeven wrote:
> [...]
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> However, recently Dominik (CC) send me an enquiry about it, so perhaps
> he is a happy user?

Nope - don't have one either, so go ahead :)


Regards,
Dominik

Patch

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 6da20b9688f7..800ad252cf9c 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1034,14 +1034,7 @@  int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
 	bool hotpluggable;
 
 	/* We are scanning "memory" nodes only */
-	if (type == NULL) {
-		/*
-		 * The longtrail doesn't have a device_type on the
-		 * /memory node, so look for the node called /memory@0.
-		 */
-		if (!IS_ENABLED(CONFIG_PPC32) || depth != 1 || strcmp(uname, "memory@0") != 0)
-			return 0;
-	} else if (strcmp(type, "memory") != 0)
+	if (type == NULL || strcmp(type, "memory") != 0)
 		return 0;
 
 	reg = of_get_flat_dt_prop(node, "linux,usable-memory", &l);