* Re: [RFC PATCH 14/19] powerpc: allow ioremap within reserved fake ram regions
From: Albert Herranz @ 2009-11-24 17:09 UTC (permalink / raw)
To: michael; +Cc: linuxppc-dev
In-Reply-To: <1259019909.31172.63.camel@concordia>
Michael Ellerman wrote:
>> Would it be acceptable to create a global var __allow_ioremap_normal_ram that by default would have a value of 0 and would be set _only_ for those platforms needing it?
>>
>> The other solutions I see is:
>> - add support for discontiguous memory to powerpc 32-bits (which is not something that I can look into now)
>> - don't use the precious second 64MB area (which is a waste)
>
> - Implement your own ppc_md.ioremap(), see iseries & cell for example.
>
> Currently that's only called on 64-bit, but you could change that.
>
> If I'm reading your patch right, you should be able to do that check in
> your platform's ioremap() and then call the generic implementation to do
> the actual work.
>
> cheers
I could use ppc_md.ioremap to duplicate ioremap except for the ioremap ram check.
But calling the stock ioremap without modifying it is not possible because it checks and bails out when ioremapping a region marked as ram (even if it's not real ram and it's memreserved).
Is the list of memreserved areas preserved once the kernel is running?
If it is preserved another option would be to unban ioremapping ram if memreserved.
Thanks for your input,
Albert
^ permalink raw reply
* Re: [PATCH 11/11] of: unify phandle name in struct device_node
From: David Miller @ 2009-11-24 17:37 UTC (permalink / raw)
To: grant.likely
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev
In-Reply-To: <20091124081957.6216.74456.stgit@angua>
From: Grant Likely <grant.likely@secretlab.ca>
Date: Tue, 24 Nov 2009 01:20:05 -0700
> In struct device_node, the phandle is named 'linux_phandle' for PowerPC
> and MicroBlaze, and 'node' for SPARC. There is no good reason for the
> difference, it is just an artifact of the code diverging over a couple
> of years. This patch renames .node to .linux_phandle for SPARC.
I know it's just a name, but there is no reason to put "linux" in the
member name. It's the real device phandle value from OpenFirmware not
something invented by Linux's OF layer.
PowerPC uses this for something different, it records the information
here using the special "linux,phandle" and "ibm,phandle" properties.
See unflatten_dt_node() in arch/powerpc/kernel/prom.c before your
changes.
^ permalink raw reply
* Re: [RFC PATCH 03/19] powerpc: gamecube: bootwrapper bits
From: Albert Herranz @ 2009-11-24 17:38 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <CE435D2F-8C67-4045-942D-4AA08BF784D0@kernel.crashing.org>
Segher Boessenkool wrote:
> Hi Albert,
>
>> +asm ("\n\
>
>
> A file scope asm?! Please don't.
>
So what's your proposal then? Placing it within a fake func?
That asm snippet is the entry point. I took as an example how prpmc2800.c deals with that, providing an own version of the (weak) _zImage_start.
>> + * We enter with the cache enabled, the MMU enabled and some known
>> legacy
>> + * memory mappings active. xBAT3 is unused
>
> It would be good if you could depend as little as possible on these things;
> that makes writing another bootloader a lot easier.
>
Ok. I'll do a similar approach as done on the wii bootwrapper.
>> + /* IBAT3,DBAT3 for first 16Mbytes */\n\
>> + li 8, 0x01ff /* 16MB */\n\
>> + li 9, 0x0002 /* rw */\n\
>> + mtspr 0x216, 8 /* IBAT3U */\n\
>> + mtspr 0x217, 9 /* IBAT3L */\n\
>> + mtspr 0x21e, 8 /* DBAT3U */\n\
>> + mtspr 0x21f, 9 /* DBAT3L */\n\
>
> WIMG=0000, are you sure? Not M=1?
>
To be honest, I don't recall the details now.
But it was tested in the very early days, the result was not the expected one and, in the end, manual cache coherency management was still needed.
So everything is designed and working assuming M=0.
This can be re-checked again later if needed.
>> + bcl- 20,4*cr7+so,1f\n\
>
> Just write bcl 20,31,1f .
Ok, I used two variants for this and I know which one you like now ;).
>
>
> Segher
>
>
Thanks,
Albert
^ permalink raw reply
* Re: [RFC PATCH 05/19] powerpc: wii: bootwrapper bits
From: Albert Herranz @ 2009-11-24 17:56 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <90646C9F-F27D-47AD-9985-83DB7381D6BF@kernel.crashing.org>
Segher Boessenkool wrote:
>> + * We enter with an unknown cache, high BATs and MMU status.
>
> What does this mean? You know the low four BATs on entry and
> nothing else?
>
That means that we do not make assumptions regarding:
- the state of the cache (enabled vs disabled)
- if the high BATs are enabled or not
- if the MMU is enabled or not
Is this clearer? I'm not a native english speaker as you may have noticed already :-P
>> +asm ("\n\
>
> Global asm() is evil.
>
Yes, you said. Still, I'd like a proper argument :)
>> + mfmsr 9\n\
>> + andi. 0, 9, (1<<4)|(1<<5) /* MSR_DR|MSR_IR */\n\
>
>> + andc 9, 9, 0\n\
>
> mfmsr 9 ; rlwinm 9,9,0,~0x30 ?
>
Yes. Maybe
mfmsr 9 ; rlwinm 9,9,0,~((1<<4)|(1<<5)) /* MSR_DR|MSR_IR */
>> + mtspr 0x01a, 8 /* SRR0 */\n\
>> + mtspr 0x01b, 9 /* SRR1 */\n\
>
> mtsrr0 and mtsrr1
>
Easier :)
>> + sync\n\
>> + rfi\n\
>
> No need for sync before rfi
>
Ok.
>> + mtspr 0x210, 8 /* IBAT0U */\n\
>> + mtspr 0x211, 8 /* IBAT0L */\n\
>
> You only need to set the upper BAT to zero, saves some code.
>
Great. Is this documented somewhere?
>> + isync\n\
>
> isync here is cargo cult
>
I'll offer a dead chicken to compensate for this.
>> + li 8, 0x01ff /* first 16MiB */\n\
>> + li 9, 0x0002 /* rw */\n\
>> + mtspr 0x210, 8 /* IBAT0U */\n\
>> + mtspr 0x211, 9 /* IBAT0L */\n\
>> + mtspr 0x218, 8 /* DBAT0U */\n\
>> + mtspr 0x219, 9 /* DBAT0L */\n\
>
> M=0 for RAM?
>
See analog question for gamecube bootwrapper bits.
>>
>
> Also, you should normally write the lower BAT first. Doesn't matter
> here because IR=DR=0 of course.
>
I can change that too if it's the general way to go.
>> + lis 8, 0xcc00 /* I/O mem */\n\
>> + ori 8, 8, 0x3ff /* 32MiB */\n\
>> + lis 9, 0x0c00\n\
>> + ori 9, 9, 0x002a /* uncached, guarded, rw */\n\
>> + mtspr 0x21a, 8 /* DBAT1U */\n\
>> + mtspr 0x21b, 9 /* DBAT1L */\n\
>
> Is there any real reason you don't identity map this?
>
No. There's a bit of nostalgia there.
(On the other hand there's no real reason to identity map it).
>> + sync\n\
>> + isync\n\
>> +\n\
>
> Don't need these
>
I'll get rid of them, then.
>> + /* enable high BATs */\n\
>> + lis 8, 0x8200\n\
>> + mtspr 0x3f3, 8 /* HID4 */\n\
>
> You need to use read-modify-write here. Also, shouldn't you
> enable the extra BATs before setting them?
>
No. You should first set them up and then enable them.
The content of the HIGH BATs is undefined on boot, AFAIK.
> And you _do_ need isync here as far as I can see.
>
>> + /* enable caches */\n\
>> + mfspr 8, 0x3f0\n\
>> + ori 8, 8, 0xc000\n\
>> + mtspr 0x3f0, 8 /* HID0 */\n\
>> + isync\n\
>
> You need to invalidate the L1 caches at the same time as you enable
> them.
>
Ok. I'll check if the caches are enabled. If they aren't I'll invalidate and enable them.
>> +void platform_init(unsigned long r3, unsigned long r4, unsigned long r5)
>> +{
>> + u32 heapsize = 24*1024*1024 - (u32)_end;
>> +
>> + simple_alloc_init(_end, heapsize, 32, 64);
>> + fdt_init(_dtb_start);
>> +
>> + if (!ug_grab_io_base() && ug_is_adapter_present())
>
> The "!" reads weird. Can you not make ug_is_adapter_present()
> call ug_grab_io_base(), anyway?
>
Calling ug_grab_io_base() from ug_is_adapter_present() can be misleading too (you are supposed to just check if the adapter is present in that function, according to the name).
If the "!" is ugly I can use the following idiom, introducing an error variable.
error = ug_grab_io_base();
if (!error && ug_is_adapter_present())
/* blah */
Thanks,
Albert
^ permalink raw reply
* Re: [RFC PATCH 06/19] powerpc: gamecube/wii: introduce GAMECUBE_COMMON
From: Albert Herranz @ 2009-11-24 18:03 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <3D2BF5CA-C4BE-4250-8EC9-7A8E767572E2@kernel.crashing.org>
Segher Boessenkool wrote:
>> Add a config option GAMECUBE_COMMON to be used as a dependency for all
>> options common to the Nintendo GameCube and Wii video game consoles.
>
> Maybe something like GAMECUBE_OR_WII instead? "COMMON" is so
> common it's meaningless.
>
I don't like either GAMECUBE_OR_WII.
It looks as if it can be used to match options for the GAMECUBE _or_ WII. But that's not the meaning of it.
This option should be used only for options applicable to both the GAMECUBE and WII, i.e. basically those options in the WII which where "inherited" from the GAMECUBE to make it retro-compatible.
If GAMECUBE_COMMON is unacceptable, maybe GAMECUBE_LEGACY or GAMECUBE_COMPAT?
Thanks,
Albert
^ permalink raw reply
* Re: [RFC PATCH 09/19] powerpc: gamecube/wii: udbg support for usbgecko
From: Albert Herranz @ 2009-11-24 18:08 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <E6251558-8B55-4A7C-952C-4989D876C69A@kernel.crashing.org>
Segher Boessenkool wrote:
>> + If you say yes to this option, support will be included for the
>> + USB Gecko adapter as an udbg console.
>> + The USB Gecko is a EXI to USB Serial converter that can be plugged
>> + into a memcard slot in the Nintendo GameCube/Wii.
>
> Not "a" memcard slot, only the first one, you have it hardcoded.
>
No. It's not hardcoded in the code. It's specified in the device tree source.
The hardcoded one is just the early udbg, not the "normal" udbg.
>> +#if 0
>> +/*
>> + * Trasmits a null terminated character string.
>> + */
>> +static void ug_puts(char *s)
>> +{
>> + while (*s)
>> + ug_putc(*s++);
>> +}
>> +#endif
>
> Remove?
>
Ok :)
>> + stdout = of_find_node_by_path(path);
>> + if (!stdout) {
>> + udbg_printf("%s: missing path %s", __func__, path);
>> + goto done;
>> + }
>> +
>> + for (np = NULL;
>> + (np = of_find_compatible_node(np, NULL, "usbgecko,usbgecko"));)
>> + if (np == stdout)
>> + break;
>> +
>> + of_node_put(stdout);
>> + if (!np) {
>> + udbg_printf("%s: stdout is not an usbgecko", __func__);
>> + goto done;
>> + }
>
> Surely there is something called something like of_node_is_compatible()
> you can use here? You already have the node pointer, there is no need
> to look at all other nodes.
>
I see the point.
I didn't find of_node_is_compatible() but I'll look what's available here.
Thanks,
Albert
^ permalink raw reply
* Re: [PATCH] ppc64: re-enable kexec to allow module loads with CONFIG_MODVERSIONS and CONFIG_RELOCATABLE turned on
From: Neil Horman @ 2009-11-24 18:11 UTC (permalink / raw)
To: linuxppc-dev; +Cc: rusty, paulus
In-Reply-To: <20091119195216.GC11414@hmsreliant.think-freely.org>
Neil Horman writes:
> Before anyone flames me for what a oddball solution this is, let me just
> say I'm trying to get the ball rolling here. I think there may be better
> solutions that can be impemented in reloc_64.S, but I've yet to make any of
the
> ones I've tried work successfully. I'm open to suggestions, but this solution
> is the only one so far that I've been able to get to work. thanks :)
I had thought that the CRCs would be the only things with reloc
addends less than 4G, but it turns out that the toolchain folds
expressions like __pa(&sym) into just a load from a doubleword (in the
TOC) containing the physical address of sym. That needs to be
relocated and its reloc addend will be less than 4GB. Hence the
crashes early in boot that you were seeing with my proposed patch to
reloc_64.S.
Given that, your solution seems as reasonable as any. Should this go
via the modules maintainer, Rusty Russell, perhaps?
In any case,
Acked-by: Paul Mackerras <paulus at samba.org>
I'm not 100% sure, it does kind of split the middle in regards to what tree is
should come through.
Rusy, given the previous entries in this thread, do you have any thoughts on the
patch, or which tree it should go through? I kind of think that the ppc tree is
just fine here since its currently the only user of this code, but I'll defer to
other opinions on the subject.
Thanks & Regards
Neil
^ permalink raw reply
* Re: [RFC PATCH 10/19] powerpc: gamecube/wii: early debugging using usbgecko
From: Albert Herranz @ 2009-11-24 18:19 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <A07AD196-D33F-477C-B53C-AB05BA236FEC@kernel.crashing.org>
Segher Boessenkool wrote:
> You set up DBAT1 here...
>
>> +setup_usbgecko_bat:
>> + /* prepare a BAT for early io */
>> + lis r8, 0x0c00
>> + ori r8, r8, 0x002a /* uncached, guarded ,rw */
>> + lis r11, 0xcc00
>> + ori r11, r11, 0x3 /* 128K */
>> +#ifdef CONFIG_WII
>> + oris r8, r8, 0x0100
>> + oris r11, r11, 0x0100
>> +#endif
>> + mtspr SPRN_DBAT1L, r8
>> + mtspr SPRN_DBAT1U, r11
>> + sync
>> + isync
>> + blr
>
> ... and again here:
>
>> +void __init udbg_init_usbgecko(void)
>> +{
>> + unsigned long vaddr, paddr;
>> +
>> +#if defined(CONFIG_GAMECUBE)
>> + paddr = 0x0c000000;
>> +#elif defined(CONFIG_WII)
>> + paddr = 0x0d000000;
>> +#else
>> +#error Invalid platform for USB Gecko based early debugging.
>> +#endif
>> +
>> + vaddr = 0xc0000000 | paddr;
>> + setbat(1, vaddr, paddr, 128*1024, PAGE_KERNEL_NCG);
>
> Do you need to do it twice?
>
Uhmm... I need to re-check it.
IIRC the BATs were re-initialized in between. But I'm not sure now :)
>> + ug_io_base = (void __iomem *)(vaddr | 0x6814);
>
> Oh, hardcoded slot2, now i'm confused which one should be it :-)
>
early udbg : hardcoded to mem2 slot
normal udbg : as specified in device tree
Clearer now ? ;)
Thanks for the review!
Albert
^ permalink raw reply
* Re: [PATCH 11/11] of: unify phandle name in struct device_node
From: Grant Likely @ 2009-11-24 20:33 UTC (permalink / raw)
To: David Miller
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev
In-Reply-To: <20091124.093732.203692950.davem@davemloft.net>
On Tue, Nov 24, 2009 at 10:37 AM, David Miller <davem@davemloft.net> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Tue, 24 Nov 2009 01:20:05 -0700
>
>> In struct device_node, the phandle is named 'linux_phandle' for PowerPC
>> and MicroBlaze, and 'node' for SPARC. =A0There is no good reason for the
>> difference, it is just an artifact of the code diverging over a couple
>> of years. =A0This patch renames .node to .linux_phandle for SPARC.
>
> I know it's just a name, but there is no reason to put "linux" in the
> member name. =A0It's the real device phandle value from OpenFirmware not
> something invented by Linux's OF layer.
I agree, and I also considered renaming it to simply 'phandle' or to
change the powerpc code back to using .node everywhere (more on .node
usage in powerpc below). I didn't want to use .node because .node is
pretty generic, and is used in other places for different things.
Basically I wanted to avoid confusion. In particular, .node points to
a device_node, not a phandle, in struct of_device. Changing all
references to simply 'phandle' seems to be the best, but it does
require changes to more locations in the code. In the end I decided
on some constructive lazyness by settling for the already-used
.linux_phandle so that I would need to touch as many files, but still
retain a unique name that is easy to find. If you prefer, I can
change it all to simply '.phandle'.
> PowerPC uses this for something different, it records the information
> here using the special "linux,phandle" and "ibm,phandle" properties.
Agreed, the phandle isn't a real phandle when using the flat tree.
However, though the source of the phandle value is different, the use
case is identical, so it make sense to stuff it into the same member.
Merging them lets me unify more code. For example,
of_find_node_by_phandle().
> See unflatten_dt_node() in arch/powerpc/kernel/prom.c before your
> changes.
Yup, I looked at this. The unflatten code stuffs both linux,phandle
and ibm,phandle into .linux_phandle, and linux,phandle also gets
stuffed into .node. But all the users except one use the
.linux_phandle, not .node, and that remaining user *I think* can
safely use .linux_phandle too.
Thanks for the review,
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [RFC PATCH 03/19] powerpc: gamecube: bootwrapper bits
From: Segher Boessenkool @ 2009-11-24 21:00 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0C1A25.8030401@yahoo.es>
>>> +asm ("\n\
>>
>>
>> A file scope asm?! Please don't.
>>
> So what's your proposal then? Placing it within a fake func?
> That asm snippet is the entry point. I took as an example how
> prpmc2800.c deals with that, providing an own version of the (weak)
> _zImage_start.
Use an assembler source file. You'll get much nicer syntax as well
(none of that \n stuff).
>>> + /* IBAT3,DBAT3 for first 16Mbytes */\n\
>>> + li 8, 0x01ff /* 16MB */\n\
>>> + li 9, 0x0002 /* rw */\n\
>>> + mtspr 0x216, 8 /* IBAT3U */\n\
>>> + mtspr 0x217, 9 /* IBAT3L */\n\
>>> + mtspr 0x21e, 8 /* DBAT3U */\n\
>>> + mtspr 0x21f, 9 /* DBAT3L */\n\
>>
>> WIMG=0000, are you sure? Not M=1?
>
> To be honest, I don't recall the details now.
> But it was tested in the very early days, the result was not the
> expected one and, in the end, manual cache coherency management was
> still needed.
Sure, the memory controllers don't do coherency. I'm slightly worried
about two things:
1) Will the generic code use M=0 as well? Is it a problem if it
doesn't?
2) Do lwarx. etc. work in M=0?
And a question: does M=0 actually give better performance (lower bus
utilisation, and maybe saves a few cycles)?
Segher
^ permalink raw reply
* Re: [RFC PATCH 05/19] powerpc: wii: bootwrapper bits
From: Segher Boessenkool @ 2009-11-24 21:13 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0C1E61.6010101@yahoo.es>
>>> + * We enter with an unknown cache, high BATs and MMU status.
>>
>> What does this mean? You know the low four BATs on entry and
>> nothing else?
>>
>
> That means that we do not make assumptions regarding:
> - the state of the cache (enabled vs disabled)
> - if the high BATs are enabled or not
> - if the MMU is enabled or not
>
> Is this clearer?
Yeah it is.
>>> + mfmsr 9\n\
>>> + andi. 0, 9, (1<<4)|(1<<5) /* MSR_DR|MSR_IR */\n\
>>
>>> + andc 9, 9, 0\n\
>>
>> mfmsr 9 ; rlwinm 9,9,0,~0x30 ?
>>
>
> Yes. Maybe
>
> mfmsr 9 ; rlwinm 9,9,0,~((1<<4)|(1<<5)) /* MSR_DR|MSR_IR */
Sure, or ~0x30 with that comment. You can save an insn and make
the code clearer at the same time, how exactly you write it, I don't
care :-)
>>> + mtspr 0x210, 8 /* IBAT0U */\n\
>>> + mtspr 0x211, 8 /* IBAT0L */\n\
>>
>> You only need to set the upper BAT to zero, saves some code.
>
> Great. Is this documented somewhere?
Sure, it's all in the PEM.
>>> + isync\n\
>>
>> isync here is cargo cult
>
> I'll offer a dead chicken to compensate for this.
Thanks, I needed some more of those :-)
>> Also, you should normally write the lower BAT first. Doesn't matter
>> here because IR=DR=0 of course.
>
> I can change that too if it's the general way to go.
Please do.
>>> + lis 8, 0xcc00 /* I/O mem */\n\
>>> + ori 8, 8, 0x3ff /* 32MiB */\n\
>>> + lis 9, 0x0c00\n\
>>> + ori 9, 9, 0x002a /* uncached, guarded, rw */\n\
>>> + mtspr 0x21a, 8 /* DBAT1U */\n\
>>> + mtspr 0x21b, 9 /* DBAT1L */\n\
>>
>> Is there any real reason you don't identity map this?
> No. There's a bit of nostalgia there.
> (On the other hand there's no real reason to identity map it).
It's a tiny bit cleaner and stops people from wondering why :-)
>>> + /* enable high BATs */\n\
>>> + lis 8, 0x8200\n\
>>> + mtspr 0x3f3, 8 /* HID4 */\n\
>>
>> You need to use read-modify-write here. Also, shouldn't you
>> enable the extra BATs before setting them?
>>
>
> No. You should first set them up and then enable them.
> The content of the HIGH BATs is undefined on boot, AFAIK.
All BATs are undefined at boot; you need to clear them / set them
before enabling them (DR=1 or IR=1), so there is nothing special
about the high BATs.
What I am getting at is if the mfspr/mtspr to the high BATs does
work when the HID bit for them is off.
Please address the RMW thing (don't clear bits in HID4).
>>> + /* enable caches */\n\
>>> + mfspr 8, 0x3f0\n\
>>> + ori 8, 8, 0xc000\n\
>>> + mtspr 0x3f0, 8 /* HID0 */\n\
>>> + isync\n\
>>
>> You need to invalidate the L1 caches at the same time as you enable
>> them.
>>
>
> Ok. I'll check if the caches are enabled. If they aren't I'll
> invalidate and enable them.
Yeah, good point.
> If the "!" is ugly I can use the following idiom, introducing an
> error variable.
>
> error = ug_grab_io_base();
> if (!error && ug_is_adapter_present())
> /* blah */
Much clearer, thanks.
Segher
^ permalink raw reply
* Re: [PATCH 11/11] of: unify phandle name in struct device_node
From: Benjamin Herrenschmidt @ 2009-11-24 21:06 UTC (permalink / raw)
To: David Miller
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev
In-Reply-To: <20091124.093732.203692950.davem@davemloft.net>
On Tue, 2009-11-24 at 09:37 -0800, David Miller wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Tue, 24 Nov 2009 01:20:05 -0700
>
> > In struct device_node, the phandle is named 'linux_phandle' for PowerPC
> > and MicroBlaze, and 'node' for SPARC. There is no good reason for the
> > difference, it is just an artifact of the code diverging over a couple
> > of years. This patch renames .node to .linux_phandle for SPARC.
>
> I know it's just a name, but there is no reason to put "linux" in the
> member name. It's the real device phandle value from OpenFirmware not
> something invented by Linux's OF layer.
>
> PowerPC uses this for something different, it records the information
> here using the special "linux,phandle" and "ibm,phandle" properties.
>
> See unflatten_dt_node() in arch/powerpc/kernel/prom.c before your
> changes.
Right, this comes from a subtle problem with our firmwares on pSeries.
We have a phandle from OF. But some devices, especially virtual devices,
also have an "ibm,phandle" which may or may not be the same afaik.
In addition, when the hypervisor hotplugs some devices, it sends us some
new device-tree bits to add. Those don't have a phandle in the formal
sense afaik, but -do- have an ibm,phandle property.
I think we can always just use ibm,phandle instead of the OF one when
the former is present, they should be non overlapping, but of course,
any chance in that area will require plenty of testing on some of those
machines.
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 06/19] powerpc: gamecube/wii: introduce GAMECUBE_COMMON
From: Segher Boessenkool @ 2009-11-24 21:15 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0C2002.4050704@yahoo.es>
>>> Add a config option GAMECUBE_COMMON to be used as a dependency
>>> for all
>>> options common to the Nintendo GameCube and Wii video game consoles.
>>
>> Maybe something like GAMECUBE_OR_WII instead? "COMMON" is so
>> common it's meaningless.
>>
>
> I don't like either GAMECUBE_OR_WII.
> It looks as if it can be used to match options for the GAMECUBE
> _or_ WII. But that's not the meaning of it.
>
> This option should be used only for options applicable to both the
> GAMECUBE and WII, i.e. basically those options in the WII which
> where "inherited" from the GAMECUBE to make it retro-compatible.
>
> If GAMECUBE_COMMON is unacceptable, maybe GAMECUBE_LEGACY or
> GAMECUBE_COMPAT?
Maybe it's best to write GAMECUBE || WII in the places that use it,
then?
Segher
^ permalink raw reply
* Re: [PATCH 11/11] of: unify phandle name in struct device_node
From: David Miller @ 2009-11-24 21:10 UTC (permalink / raw)
To: grant.likely
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev
In-Reply-To: <fa686aa40911241233k7c6d428g8f70f00abdd754e8@mail.gmail.com>
From: Grant Likely <grant.likely@secretlab.ca>
Date: Tue, 24 Nov 2009 13:33:22 -0700
> If you prefer, I can change it all to simply '.phandle'.
Please do.
^ permalink raw reply
* Re: [RFC PATCH 09/19] powerpc: gamecube/wii: udbg support for usbgecko
From: Segher Boessenkool @ 2009-11-24 21:19 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0C2115.3010006@yahoo.es>
>>> + If you say yes to this option, support will be included
>>> for the
>>> + USB Gecko adapter as an udbg console.
>>> + The USB Gecko is a EXI to USB Serial converter that can be
>>> plugged
>>> + into a memcard slot in the Nintendo GameCube/Wii.
>>
>> Not "a" memcard slot, only the first one, you have it hardcoded.
>>
>
> No. It's not hardcoded in the code. It's specified in the device
> tree source.
> The hardcoded one is just the early udbg, not the "normal" udbg.
Ah I misread the code then.
>> Surely there is something called something like
>> of_node_is_compatible()
>> you can use here? You already have the node pointer, there is no
>> need
>> to look at all other nodes.
>>
>
> I see the point.
> I didn't find of_node_is_compatible() but I'll look what's
> available here.
int of_device_is_compatible(const struct device_node *device,
const char *compat)
Segher
^ permalink raw reply
* Re: [RFC PATCH 11/19] powerpc: gamecube/wii: flipper interrupt controller support
From: Segher Boessenkool @ 2009-11-24 21:30 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <1258927311-4340-12-git-send-email-albert_herranz@yahoo.es>
> config GAMECUBE_COMMON
> bool
> select NOT_COHERENT_CACHE
> + select FLIPPER_PIC
Maybe using FLIPPER (or GAMECUBE_FLIPPER) instead of GAMECUBE_COMMON
is a good name?
> +#define pr_fmt(fmt) DRV_MODULE_NAME ": " fmt
Unused
> +/*
> + * Each interrupt has a corresponding bit in both
> + * the Interrupt Cause (ICR) and Interrupt Mask (IMR) registers.
> + *
> + * Enabling/disabling an interrupt line involves asserting/clearing
> + * the corresponding bit in IMR. ACK'ing a request simply involves
> + * asserting the corresponding bit in ICR.
> + */
> +static void flipper_pic_ack(unsigned int virq)
> +{
> + int irq = virq_to_hw(virq);
> + void __iomem *io_base = get_irq_chip_data(virq);
> +
> + set_bit(irq, io_base + FLIPPER_ICR);
> +}
So it should be a simple write instead of an RMW here, right?
As it is you are ACKing _all_ IRQs as far as I can see.
> +unsigned int flipper_pic_get_irq(void)
> +{
> + void __iomem *io_base = flipper_irq_host->host_data;
> + int irq;
> + u32 irq_status;
> +
> + irq_status = in_be32(io_base + FLIPPER_ICR) &
> + in_be32(io_base + FLIPPER_IMR);
> + if (irq_status == 0)
> + return -1; /* no more IRQs pending */
> +
> + __asm__ __volatile__("cntlzw %0,%1" : "=r"(irq) : "r"(irq_status));
> + return irq_linear_revmap(flipper_irq_host, 31 - irq);
> +}
There are generic macros for this kind of thing, no need for asm. ffs()
or something.
Segher
^ permalink raw reply
* Re: [RFC PATCH 12/19] powerpc: gamecube: platform support
From: Segher Boessenkool @ 2009-11-24 21:39 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <1258927311-4340-13-git-send-email-albert_herranz@yahoo.es>
> +static struct of_device_id gamecube_of_bus[] = {
> + { .compatible = "nintendo,flipper", },
> + { },
> +};
> +
> +static int __init gamecube_device_probe(void)
> +{
> + if (!machine_is(gamecube))
> + return 0;
> +
> + of_platform_bus_probe(NULL, gamecube_of_bus, NULL);
> + return 0;
> +}
You really do not need a platform bus as far as I can see?
Segher
^ permalink raw reply
* Re: [PATCH 11/11] of: unify phandle name in struct device_node
From: Grant Likely @ 2009-11-24 21:39 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, David Miller
In-Reply-To: <1259096815.16367.133.camel@pasglop>
On Tue, Nov 24, 2009 at 2:06 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2009-11-24 at 09:37 -0800, David Miller wrote:
>> From: Grant Likely <grant.likely@secretlab.ca>
>> Date: Tue, 24 Nov 2009 01:20:05 -0700
>>
>> > In struct device_node, the phandle is named 'linux_phandle' for PowerP=
C
>> > and MicroBlaze, and 'node' for SPARC. =A0There is no good reason for t=
he
>> > difference, it is just an artifact of the code diverging over a couple
>> > of years. =A0This patch renames .node to .linux_phandle for SPARC.
>>
>> I know it's just a name, but there is no reason to put "linux" in the
>> member name. =A0It's the real device phandle value from OpenFirmware not
>> something invented by Linux's OF layer.
>>
>> PowerPC uses this for something different, it records the information
>> here using the special "linux,phandle" and "ibm,phandle" properties.
>>
>> See unflatten_dt_node() in arch/powerpc/kernel/prom.c before your
>> changes.
>
> Right, this comes from a subtle problem with our firmwares on pSeries.
>
> We have a phandle from OF. But some devices, especially virtual devices,
> also have an "ibm,phandle" which may or may not be the same afaik.
>
> In addition, when the hypervisor hotplugs some devices, it sends us some
> new device-tree bits to add. Those don't have a phandle in the formal
> sense afaik, but -do- have an ibm,phandle property.
>
> I think we can always just use ibm,phandle instead of the OF one when
> the former is present, they should be non overlapping, but of course,
> any chance in that area will require plenty of testing on some of those
> machines.
If pseries is the troublesome platform, not powermac, then I'm pretty
confident this change is okay. I cannot find any other users of .node
other than arch/powerpc/platforms/powermac/pfunc_core.c, but I'll
double check with an allmodconfig and an allyesconfig. This patch
shouldn't change the behaviour of linux_phandle at all.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [RFC PATCH 16/19] powerpc: wii: hollywood interrupt controller support
From: Segher Boessenkool @ 2009-11-24 21:54 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <1258927311-4340-17-git-send-email-albert_herranz@yahoo.es>
> +config HLWD_PIC
Are vowels too expensive? :-)
> +static void hlwd_pic_ack(unsigned int virq)
> +{
> + int irq = virq_to_hw(virq);
> + void __iomem *io_base = get_irq_chip_data(virq);
> +
> + set_bit(irq, io_base + HW_BROADWAY_ICR);
> +}
Same issue as with Flipper here.
> + __asm__ __volatile__("cntlzw %0,%1" : "=r"(irq) : "r"(irq_status));
> + return irq_linear_revmap(h, 31 - irq);
And here.
> +static void __hlwd_quiesce(void __iomem *io_base)
> +{
> + /* mask and ack all IRQs */
> + out_be32(io_base + HW_BROADWAY_IMR, 0);
> + out_be32(io_base + HW_BROADWAY_ICR, ~0);
> +}
I would write 0xffffffff instead, it's clearer and slightly
more robust.
> +void hlwd_pic_probe(void)
> +{
> + struct irq_host *host;
> + struct device_node *np;
> + const u32 *interrupts;
> + int cascade_virq;
> +
> + for_each_compatible_node(np, NULL, "nintendo,hollywood-pic") {
> + interrupts = of_get_property(np, "interrupts", NULL);
> + if (interrupts) {
> + host = hlwd_pic_init(np);
> + BUG_ON(!host);
> + cascade_virq = irq_of_parse_and_map(np, 0);
> + set_irq_data(cascade_virq, host);
> + set_irq_chained_handler(cascade_virq,
> + hlwd_pic_irq_cascade);
break; here? You do not handle more than one hollywood-pic elsewhere
(which of course is fine).
Segher
^ permalink raw reply
* [PATCH v2 11/11] of: unify phandle name in struct device_node
From: Grant Likely @ 2009-11-24 22:01 UTC (permalink / raw)
To: linuxppc-dev, devicetree-discuss, sparclinux, microblaze-uclinux,
benh, sfr, davem, monstr
In struct device_node, the phandle is named 'linux_phandle' for PowerPC
and MicroBlaze, and 'node' for SPARC. There is no good reason for the
difference, it is just an artifact of the code diverging over a couple
of years. This patch renames both to simply .phandle.
Note: the .node also existed in PowerPC/MicroBlaze, but the only user
seems to be arch/powerpc/platforms/powermac/pfunc_core.c. It doesn't
look like the assignment between .linux_phandle and .node is
significantly different enough to warrant the separate code paths
unless ibm,phandle properties actually appear in Apple device trees.
I think it is safe to eliminate the old .node property and use
linux_phandle everywhere.
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
arch/microblaze/kernel/of_platform.c | 2 +-
arch/microblaze/kernel/prom.c | 2 +-
arch/powerpc/kernel/of_platform.c | 2 +-
arch/powerpc/kernel/prom.c | 6 +++---
arch/powerpc/platforms/cell/spu_manage.c | 6 +++---
arch/powerpc/platforms/powermac/pfunc_core.c | 2 +-
arch/sparc/kernel/devices.c | 2 +-
arch/sparc/kernel/of_device_32.c | 2 +-
arch/sparc/kernel/of_device_64.c | 2 +-
arch/sparc/kernel/prom_common.c | 8 ++++----
arch/sparc/kernel/smp_64.c | 2 +-
drivers/of/fdt.c | 7 +++----
drivers/sbus/char/openprom.c | 10 +++++-----
drivers/video/aty/atyfb_base.c | 2 +-
include/linux/of.h | 5 +----
sound/aoa/fabrics/layout.c | 2 +-
16 files changed, 29 insertions(+), 33 deletions(-)
diff --git a/arch/microblaze/kernel/of_platform.c b/arch/microblaze/kernel/of_platform.c
index acf4574..1c6d684 100644
--- a/arch/microblaze/kernel/of_platform.c
+++ b/arch/microblaze/kernel/of_platform.c
@@ -185,7 +185,7 @@ EXPORT_SYMBOL(of_find_device_by_node);
static int of_dev_phandle_match(struct device *dev, void *data)
{
phandle *ph = data;
- return to_of_device(dev)->node->linux_phandle == *ph;
+ return to_of_device(dev)->node->phandle == *ph;
}
struct of_device *of_find_device_by_phandle(phandle ph)
diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
index 1f79e4d..9b4a20b 100644
--- a/arch/microblaze/kernel/prom.c
+++ b/arch/microblaze/kernel/prom.c
@@ -251,7 +251,7 @@ struct device_node *of_find_node_by_phandle(phandle handle)
read_lock(&devtree_lock);
for (np = allnodes; np != NULL; np = np->allnext)
- if (np->linux_phandle == handle)
+ if (np->phandle == handle)
break;
of_node_get(np);
read_unlock(&devtree_lock);
diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
index 1a4fc0d..666d08d 100644
--- a/arch/powerpc/kernel/of_platform.c
+++ b/arch/powerpc/kernel/of_platform.c
@@ -214,7 +214,7 @@ EXPORT_SYMBOL(of_find_device_by_node);
static int of_dev_phandle_match(struct device *dev, void *data)
{
phandle *ph = data;
- return to_of_device(dev)->node->linux_phandle == *ph;
+ return to_of_device(dev)->node->phandle == *ph;
}
struct of_device *of_find_device_by_phandle(phandle ph)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 7d0beeb..d3ce311 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -701,7 +701,7 @@ struct device_node *of_find_node_by_phandle(phandle handle)
read_lock(&devtree_lock);
for (np = allnodes; np != 0; np = np->allnext)
- if (np->linux_phandle == handle)
+ if (np->phandle == handle)
break;
of_node_get(np);
read_unlock(&devtree_lock);
@@ -771,9 +771,9 @@ static int of_finish_dynamic_node(struct device_node *node)
if (machine_is(powermac))
return -ENODEV;
- /* fix up new node's linux_phandle field */
+ /* fix up new node's phandle field */
if ((ibm_phandle = of_get_property(node, "ibm,phandle", NULL)))
- node->linux_phandle = *ibm_phandle;
+ node->phandle = *ibm_phandle;
out:
of_node_put(parent);
diff --git a/arch/powerpc/platforms/cell/spu_manage.c b/arch/powerpc/platforms/cell/spu_manage.c
index 4c506c1..891f18e 100644
--- a/arch/powerpc/platforms/cell/spu_manage.c
+++ b/arch/powerpc/platforms/cell/spu_manage.c
@@ -457,7 +457,7 @@ neighbour_spu(int cbe, struct device_node *target, struct device_node *avoid)
continue;
vic_handles = of_get_property(spu_dn, "vicinity", &lenp);
for (i=0; i < (lenp / sizeof(phandle)); i++) {
- if (vic_handles[i] == target->linux_phandle)
+ if (vic_handles[i] == target->phandle)
return spu;
}
}
@@ -499,7 +499,7 @@ static void init_affinity_node(int cbe)
if (strcmp(name, "spe") == 0) {
spu = devnode_spu(cbe, vic_dn);
- avoid_ph = last_spu_dn->linux_phandle;
+ avoid_ph = last_spu_dn->phandle;
} else {
/*
* "mic-tm" and "bif0" nodes do not have
@@ -514,7 +514,7 @@ static void init_affinity_node(int cbe)
last_spu->has_mem_affinity = 1;
spu->has_mem_affinity = 1;
}
- avoid_ph = vic_dn->linux_phandle;
+ avoid_ph = vic_dn->phandle;
}
list_add_tail(&spu->aff_list, &last_spu->aff_list);
diff --git a/arch/powerpc/platforms/powermac/pfunc_core.c b/arch/powerpc/platforms/powermac/pfunc_core.c
index 96d5ce5..ede49e7 100644
--- a/arch/powerpc/platforms/powermac/pfunc_core.c
+++ b/arch/powerpc/platforms/powermac/pfunc_core.c
@@ -842,7 +842,7 @@ struct pmf_function *__pmf_find_function(struct device_node *target,
list_for_each_entry(func, &dev->functions, link) {
if (name && strcmp(name, func->name))
continue;
- if (func->phandle && target->node != func->phandle)
+ if (func->phandle && target->phandle != func->phandle)
continue;
if ((func->flags & flags) == 0)
continue;
diff --git a/arch/sparc/kernel/devices.c b/arch/sparc/kernel/devices.c
index b171ae8..b062de9 100644
--- a/arch/sparc/kernel/devices.c
+++ b/arch/sparc/kernel/devices.c
@@ -59,7 +59,7 @@ static int __cpu_find_by(int (*compare)(int, int, void *), void *compare_arg,
cur_inst = 0;
for_each_node_by_type(dp, "cpu") {
- int err = check_cpu_node(dp->node, &cur_inst,
+ int err = check_cpu_node(dp->phandle, &cur_inst,
compare, compare_arg,
prom_node, mid);
if (!err) {
diff --git a/arch/sparc/kernel/of_device_32.c b/arch/sparc/kernel/of_device_32.c
index 4c26eb5..09138d4 100644
--- a/arch/sparc/kernel/of_device_32.c
+++ b/arch/sparc/kernel/of_device_32.c
@@ -433,7 +433,7 @@ build_resources:
if (!parent)
dev_set_name(&op->dev, "root");
else
- dev_set_name(&op->dev, "%08x", dp->node);
+ dev_set_name(&op->dev, "%08x", dp->phandle);
if (of_device_register(op)) {
printk("%s: Could not register of device.\n",
diff --git a/arch/sparc/kernel/of_device_64.c b/arch/sparc/kernel/of_device_64.c
index 881947e..036f18a 100644
--- a/arch/sparc/kernel/of_device_64.c
+++ b/arch/sparc/kernel/of_device_64.c
@@ -666,7 +666,7 @@ static struct of_device * __init scan_one_device(struct device_node *dp,
if (!parent)
dev_set_name(&op->dev, "root");
else
- dev_set_name(&op->dev, "%08x", dp->node);
+ dev_set_name(&op->dev, "%08x", dp->phandle);
if (of_device_register(op)) {
printk("%s: Could not register of device.\n",
diff --git a/arch/sparc/kernel/prom_common.c b/arch/sparc/kernel/prom_common.c
index d80a65d..5832e13 100644
--- a/arch/sparc/kernel/prom_common.c
+++ b/arch/sparc/kernel/prom_common.c
@@ -42,7 +42,7 @@ struct device_node *of_find_node_by_phandle(phandle handle)
struct device_node *np;
for (np = allnodes; np; np = np->allnext)
- if (np->node == handle)
+ if (np->phandle == handle)
break;
return np;
@@ -89,7 +89,7 @@ int of_set_property(struct device_node *dp, const char *name, void *val, int len
void *old_val = prop->value;
int ret;
- ret = prom_setprop(dp->node, name, val, len);
+ ret = prom_setprop(dp->phandle, name, val, len);
err = -EINVAL;
if (ret >= 0) {
@@ -236,7 +236,7 @@ static struct device_node * __init prom_create_node(phandle node,
dp->name = get_one_property(node, "name");
dp->type = get_one_property(node, "device_type");
- dp->node = node;
+ dp->phandle = node;
dp->properties = build_prop_list(node);
@@ -313,7 +313,7 @@ void __init prom_build_devicetree(void)
nextp = &allnodes->allnext;
allnodes->child = prom_build_tree(allnodes,
- prom_getchild(allnodes->node),
+ prom_getchild(allnodes->phandle),
&nextp);
of_console_init();
diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
index aa36223..eb14844 100644
--- a/arch/sparc/kernel/smp_64.c
+++ b/arch/sparc/kernel/smp_64.c
@@ -370,7 +370,7 @@ static int __cpuinit smp_boot_one_cpu(unsigned int cpu)
} else {
struct device_node *dp = of_find_node_by_cpuid(cpu);
- prom_startcpu(dp->node, entry, cookie);
+ prom_startcpu(dp->phandle, entry, cookie);
}
for (timeout = 0; timeout < 50000; timeout++) {
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 6164781..13c69e0 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -315,12 +315,11 @@ unsigned long __init unflatten_dt_node(unsigned long mem,
__alignof__(struct property));
if (allnextpp) {
if (strcmp(pname, "linux,phandle") == 0) {
- np->node = *((u32 *)*p);
- if (np->linux_phandle == 0)
- np->linux_phandle = np->node;
+ if (np->phandle == 0)
+ np->phandle = *((u32 *)*p);
}
if (strcmp(pname, "ibm,phandle") == 0)
- np->linux_phandle = *((u32 *)*p);
+ np->phandle = *((u32 *)*p);
pp->name = pname;
pp->length = sz;
pp->value = (void *)*p;
diff --git a/drivers/sbus/char/openprom.c b/drivers/sbus/char/openprom.c
index 75ac19b..fc2f676 100644
--- a/drivers/sbus/char/openprom.c
+++ b/drivers/sbus/char/openprom.c
@@ -233,7 +233,7 @@ static int opromnext(void __user *argp, unsigned int cmd, struct device_node *dp
ph = 0;
if (dp)
- ph = dp->node;
+ ph = dp->phandle;
data->current_node = dp;
*((int *) op->oprom_array) = ph;
@@ -256,7 +256,7 @@ static int oprompci2node(void __user *argp, struct device_node *dp, struct openp
dp = pci_device_to_OF_node(pdev);
data->current_node = dp;
- *((int *)op->oprom_array) = dp->node;
+ *((int *)op->oprom_array) = dp->phandle;
op->oprom_size = sizeof(int);
err = copyout(argp, op, bufsize + sizeof(int));
@@ -273,7 +273,7 @@ static int oprompath2node(void __user *argp, struct device_node *dp, struct open
dp = of_find_node_by_path(op->oprom_array);
if (dp)
- ph = dp->node;
+ ph = dp->phandle;
data->current_node = dp;
*((int *)op->oprom_array) = ph;
op->oprom_size = sizeof(int);
@@ -540,7 +540,7 @@ static int opiocgetnext(unsigned int cmd, void __user *argp)
}
}
if (dp)
- nd = dp->node;
+ nd = dp->phandle;
if (copy_to_user(argp, &nd, sizeof(phandle)))
return -EFAULT;
@@ -570,7 +570,7 @@ static int openprom_bsd_ioctl(struct inode * inode, struct file * file,
case OPIOCGETOPTNODE:
BUILD_BUG_ON(sizeof(phandle) != sizeof(int));
- if (copy_to_user(argp, &options_node->node, sizeof(phandle)))
+ if (copy_to_user(argp, &options_node->phandle, sizeof(phandle)))
return -EFAULT;
return 0;
diff --git a/drivers/video/aty/atyfb_base.c b/drivers/video/aty/atyfb_base.c
index 913b4a4..bb20987 100644
--- a/drivers/video/aty/atyfb_base.c
+++ b/drivers/video/aty/atyfb_base.c
@@ -3104,7 +3104,7 @@ static int __devinit atyfb_setup_sparc(struct pci_dev *pdev,
}
dp = pci_device_to_OF_node(pdev);
- if (node == dp->node) {
+ if (node == dp->phandle) {
struct fb_var_screeninfo *var = &default_var;
unsigned int N, P, Q, M, T, R;
u32 v_total, h_total;
diff --git a/include/linux/of.h b/include/linux/of.h
index 0a51742..bbf0e58 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -39,10 +39,7 @@ struct of_irq_controller;
struct device_node {
const char *name;
const char *type;
- phandle node;
-#if !defined(CONFIG_SPARC)
- phandle linux_phandle;
-#endif
+ phandle phandle;
char *full_name;
struct property *properties;
diff --git a/sound/aoa/fabrics/layout.c b/sound/aoa/fabrics/layout.c
index 586965f..7a437da 100644
--- a/sound/aoa/fabrics/layout.c
+++ b/sound/aoa/fabrics/layout.c
@@ -768,7 +768,7 @@ static int check_codec(struct aoa_codec *codec,
"required property %s not present\n", propname);
return -ENODEV;
}
- if (*ref != codec->node->linux_phandle) {
+ if (*ref != codec->node->phandle) {
printk(KERN_INFO "snd-aoa-fabric-layout: "
"%s doesn't match!\n", propname);
return -ENODEV;
^ permalink raw reply related
* Re: [RFC PATCH 17/19] powerpc: wii: bootmii starlet 'mini' firmware support
From: Segher Boessenkool @ 2009-11-24 22:13 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <1258927311-4340-18-git-send-email-albert_herranz@yahoo.es>
> Add support for the BootMii 'mini' firmware replacement for the
> Starlet processor.
>
> 'mini' is an open source IOS replacement written from scratch by
> Team Twiizers.
It's not a replacement, it doesn't have any of the same functionality.
> It grants full access to the hardware found on the
> Nintendo Wii video game console via a custom IPC mechanism.
This is out of date, you get full register-level hardware access now.
Do you want to mainline any of the drivers that work via mini proxy?
If not, you can remove quite some bit of code here I think.
> +enum starlet_ipc_flavour {
> + STARLET_IPC_IOS,
> + STARLET_IPC_MINI,
> +};
I thought you don't support IOS at all anymore?
> +config STARLET_MINI
> + bool "BootMii Starlet 'mini' firmware support"
> + depends on WII && EXPERIMENTAL
Given that this is the only supported starlet firmware interface,
does it make sense to expose the option to the user?
<big snip, see my "remove stuff" comment above>
> +static void mipc_init_ahbprot(struct mipc_device *ipc_dev)
> +{
> + void __iomem *hw_ahbprot;
> + u32 initial_ahbprot, ahbprot = 0;
> +
> + hw_ahbprot = mipc_get_hw_reg(HW_AHBPROT_OF_COMPATIBLE);
> + if (!hw_ahbprot)
> + goto done;
> +
> + initial_ahbprot = mipc_readl(hw_ahbprot);
> + if (initial_ahbprot != 0xffffffff) {
> + pr_debug("AHBPROT=%08X (before)\n", initial_ahbprot);
> + /* enable full access from the PowerPC side */
> + mipc_writel(0xffffffff, hw_ahbprot);
> + }
> +
> + ahbprot = mipc_readl(hw_ahbprot);
> + if (initial_ahbprot != ahbprot)
> + pr_debug("AHBPROT=%08X (after)\n", ahbprot);
> +done:
> + if (ahbprot != 0xffffffff)
> + pr_err("failed to set AHBPROT\n");
> +}
Modern mini will always have AHBPROT set up to give you full access,
so this isn't needed either.
> +#ifdef CONFIG_HLWD_PIC
> + /*
> + * Setup the Hollywood interrupt controller as soon as
> + * we detect that we are running under the mini firmware.
> + */
> + hlwd_pic_probe();
> +#endif
Why? The comment doesn't say.
Do you need this driver to boot? If not, it might be best to leave it
out for now.
Segher
^ permalink raw reply
* Re: [RFC PATCH 18/19] powerpc: wii: platform support
From: Segher Boessenkool @ 2009-11-24 22:24 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <1258927311-4340-19-git-send-email-albert_herranz@yahoo.es>
> diff --git a/arch/powerpc/platforms/embedded6xx/wii.c b/arch/
> powerpc/platforms/embedded6xx/wii.c
> +#define DRV_MODULE_NAME "rvl"
Should this be "wii"?
> +static enum starlet_ipc_flavour starlet_ipc_flavour;
> +
> +enum starlet_ipc_flavour starlet_get_ipc_flavour(void)
> +{
> + return starlet_ipc_flavour;
> +}
This can go I think, unless you plan on supporting something else
than mini?
> +#ifdef CONFIG_STARLET_MINI
> +
> +#define HW_RESETS_OF_COMPATIBLE "nintendo,hollywood-resets"
> +#define HW_GPIO_ALIAS "hw_gpio
This should be unconditional now I think? You access the hardware
directly.
> + np = of_find_node_by_name(NULL, "aliases");
> + if (!np) {
> + pr_err("unable to find node %s\n", "aliases");
> + goto out;
> + }
> +
> + path = of_get_property(np, HW_GPIO_ALIAS, NULL);
> + of_node_put(np);
> + if (!path) {
> + pr_err("alias %s unknown\n", HW_GPIO_ALIAS);
> + goto out;
> + }
Don't use an alias here, search for e.g. a matching "compatible"
instead.
> +static struct of_device_id wii_of_bus[] = {
> + { .compatible = "nintendo,hollywood", },
Like with Flipper, why a platform bus?
> +#ifdef CONFIG_STARLET_MINI
> + { .compatible = "twiizers,starlet-mini-ipc", },
> +#endif
This one should go completely, I will have more to say about it when
I get
to review the device trees (saving those for last :-) ).
Segher
^ permalink raw reply
* Re: [PATCH v2 11/11] of: unify phandle name in struct device_node
From: David Miller @ 2009-11-24 22:22 UTC (permalink / raw)
To: grant.likely
Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev
In-Reply-To: <20091124215908.559.23998.stgit@angua>
From: Grant Likely <grant.likely@secretlab.ca>
Date: Tue, 24 Nov 2009 15:01:14 -0700
> In struct device_node, the phandle is named 'linux_phandle' for PowerPC
> and MicroBlaze, and 'node' for SPARC. There is no good reason for the
> difference, it is just an artifact of the code diverging over a couple
> of years. This patch renames both to simply .phandle.
>
> Note: the .node also existed in PowerPC/MicroBlaze, but the only user
> seems to be arch/powerpc/platforms/powermac/pfunc_core.c. It doesn't
> look like the assignment between .linux_phandle and .node is
> significantly different enough to warrant the separate code paths
> unless ibm,phandle properties actually appear in Apple device trees.
>
> I think it is safe to eliminate the old .node property and use
> linux_phandle everywhere.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [RFC PATCH 02/19] powerpc: gamecube: device tree
From: Segher Boessenkool @ 2009-11-24 22:36 UTC (permalink / raw)
To: Grant Likely; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <fa686aa40911221502g2de254d2o4341d9abed0cdf41@mail.gmail.com>
>> + model = "NintendoGameCube";
>> + compatible = "nintendo,gamecube";
>
> To date, we've been using the same form for both the model and
> compatible properties. Specifically the <vendor>,<model> form to
> maintain the namespace.
That, however, is a) useless; and b) not totally correct.
The "model" property should show an exact model number if available.
You should use the "vendor," thing indeed.
If you don't have any better model # to use, using the same thing
as "compatible" is okay I suppose. But it's not the _best_ thing
to put here if you have the choice. Maybe you should say
"nintendo,RVL-001"
(well, that's Wii, you want the similar thing for NGC, but you get
the point I hope).
>> + compatible = "nintendo,flipper";
>> + clock-frequency = <162000000>; /* 162MHz */
>> + ranges = <0x0c000000 0x0c000000 0x00010000>;
>
> Since you're only doing 1:1 mappings; you could replace this with an
> empty "ranges;" property instead.
You could, but being explicit about the supported ranges isn't
bad either.
> Hint: If you move the interrupt-parent property up to the root node,
> then you don't need to specify it in every single device node; it will
> just inherit from the parent.
If you have only one interrupt controller, like here, you don't
need to refer to it _at all_ :-)
Segher
^ permalink raw reply
* Re: [RFC PATCH 06/19] powerpc: gamecube/wii: introduce GAMECUBE_COMMON
From: Arnd Bergmann @ 2009-11-24 22:33 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Albert Herranz
In-Reply-To: <7129FD29-82CE-4731-90F2-0EA6775EF861@kernel.crashing.org>
On Tuesday 24 November 2009, Segher Boessenkool wrote:
> > This option should be used only for options applicable to both the
> > GAMECUBE and WII, i.e. basically those options in the WII which
> > where "inherited" from the GAMECUBE to make it retro-compatible.
> >
> > If GAMECUBE_COMMON is unacceptable, maybe GAMECUBE_LEGACY or
> > GAMECUBE_COMPAT?
>
> Maybe it's best to write GAMECUBE || WII in the places that use it,
> then?
Or just use CONFIG_NINTENDO? If we ever want to port to a future
Nintendo system that is not compatible, we can still change it.
Arnd <><
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox