* Re: [RFC PATCH 16/19] powerpc: wii: hollywood interrupt controller support
From: Benjamin Herrenschmidt @ 2009-11-26 21:13 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0EA1E8.6070603@yahoo.es>
On Thu, 2009-11-26 at 16:42 +0100, Albert Herranz wrote:
> Benjamin Herrenschmidt wrote:
> > On Sun, 2009-11-22 at 23:01 +0100, Albert Herranz wrote:
> >
> >> +static void hlwd_pic_mask_and_ack(unsigned int virq)
> >> +{
> >> + int irq = virq_to_hw(virq);
> >> + void __iomem *io_base = get_irq_chip_data(virq);
> >> +
> >> + clear_bit(irq, io_base + HW_BROADWAY_IMR);
> >> + set_bit(irq, io_base + HW_BROADWAY_ICR);
> >> +}
> >
> > Same comment as with flipper. BTW. It looks really similar, can't you
> > use the same driver ?
> >
>
> I'll look at it. Surely, parts of it would be mergeable.
If it's too much work, we can do that later. No big deal.
Cheers,
Ben.
> > Cheers,
> > Ben.
> >
>
> Thanks,
> Albert
>
> >
> >
^ permalink raw reply
* Re: [RFC] powerpc/mm: honor O_SYNC flag for memory map
From: Segher Boessenkool @ 2009-11-26 21:26 UTC (permalink / raw)
To: Li Yang; +Cc: linuxppc-dev, Li Yang-R58472
In-Reply-To: <2a27d3730911250007s62f31673p3358631996e8a451@mail.gmail.com>
> So what you are saying is that if the kernel has mapped a physical
> page as cacheable while user application is trying to map it as
> non-cacheable, there will be machine checks and checkstops rather than
> just performance drop? This is new to me. Could you elaborate a bit?
If some data is in cache at a certain physical address, and you then
do an uncached read from that address, on at least some CPUs both the
bus interface and the cache will reply. Bang, machine check.
Writes are problematic as well.
Segher
^ permalink raw reply
* Re: [RFC PATCH 04/19] powerpc: wii: device tree
From: Albert Herranz @ 2009-11-26 21:20 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1259268528.18084.2.camel@pasglop>
Benjamin Herrenschmidt wrote:
> On Thu, 2009-11-26 at 16:09 +0100, Albert Herranz wrote:
>
>>> Are the above OHCI and EHCI "special" ? If not, there's an existing
>>> binding for that sort of thing, which btw requires properties to
>>> indicate the endianness of the registers and in-memory data structures
>>> etc...
>>>
>> They are a bit special because registers are in reverse little endian format,
>> must be written in 32-bit chunks, and (all things point to) they have hardware bugs.
>
> Well.. first what is "reverse little endian" ? :-) Big endian ?
>
It's the same case as the register access for the Freescale eSDHC controller, which is already in the existing sdhci-of driver.
> The OHCI driver today has separate flags to control register endianness
> and memory based data structures endianness.
>
> I think we also only use 32-bit reads and writes no ? So that should be
> covered :-)
>
There are 8, 16 and 32 bit accesses. We need the same workarounds as present for the Freescale eSDHC controller.
> As for HW bugs, well, as long as we know them we can define a quirk bit
> and add the necessary workarounds to the driver :-)
>
Yes, that's how it was done.
Although it it currently not integrated into sdhci-of. I think that the specific eSDHC code should be moved to another file, and leave the sdhci-of with the generic code and the sdhci_of_match table. Then adding a similar driver would simply consist in adding another file with the sdhci_data (containing ops and quirks) and adding an entry to the sdhci_of_match table in sdhci-of.
> Do you have a patch somewhere that adds the OCHI and EHCI support btw ?
>
Yes, too. Although it needs review and cleanup too.
I have a git tree with all the stuff, but it contains only the existing code previous to the effort of mainlining it:
http://git.infradead.org/users/herraa1/gc-linux-2.6.git/shortlog/refs/heads/mini/gc-linux-2.6.31
> Cheers,
> Ben.
>
Thanks,
Albert
^ permalink raw reply
* Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef
From: Segher Boessenkool @ 2009-11-26 21:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: sfr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <1259213266.16367.274.camel@pasglop>
>> You're right, it's not, but makes merging less complex, and then I
>> can
>> refactor properly.
>
> Still, make them __be32 at least
There is no alignment guarantee at all either, better make it all u8
and use accessor functions everywhere.
Segher
^ permalink raw reply
* Re: [RFC PATCH 04/19] powerpc: wii: device tree
From: Albert Herranz @ 2009-11-26 21:38 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1259269265.18084.14.camel@pasglop>
Benjamin Herrenschmidt wrote:
>>> Good point. I can't even guarantee that the kernel will work reliably
>>> with nobats :-) At least you really want the kernel .text to be fully
>>> covered by an IBAT.
>>>
>> It works with nobats.
>
> But does it work -reliably- ? :-)
>
It does AFAICT. My Wii is a 24x7 linux box although it is not stressed in any way usually.
> Any ways, not a big deal right now, as I said, we really want the BATs
> for performances anyways, so we should probably just add some kind of
> hack in mmu_mapin_ram() for the time being.
>
Yup. The idea is to map the first 16MB of MEM1 with a BAT.
Mapping the whole 24MB with BATs may not be possible because we want the framebuffer in MEM1 for performance reasons.
>> I must say that all the patches posted (and the device drivers, which haven't
>> been posted yet) are tested and working code.
>
> That was my impression too, but in this case, I'm talking about a
> potential very hard to hit problem that you may well have never managed
> to actually trigger.
>
I haven't actually, if that applies :)
>> There you can find the hardware interface that supports the IPC mechanism.
>> It is made up of a pair of registers to pass data between the processors and a
>> pair of control/flags registers.
>> The hardware can interrupt the PowerPC side when there is data available for it.
>
> Ok. So the right way to do that would be to have a node purely
> representing the HW IPC, unrelated to whatever is running on the
> secondary processor.
>
Totally agreed.
> However, it's ok to have -below- that node, a set of device nodes or a
> node with properties or whatever representing the FW in there and the
> function it exposes.
>
> That can be discussed later tho. I'm not that keen on having those info
> be in the .dts coming with the kernel since those functions essentially
> depend on what FW is loaded on the aux. processor.
>
I think we can leave this for later as you said.
> What might do however is to have a way for that FW itself to provide you
> with the nodes and properties for the functions it provides :-) Then you
> can have the boot wrapper or the kernel platform init code use some well
> defined (and as stable as possible) IPC API to identify the FW there and
> expose all that stuff.
>
Segher was playing with an OF implementation...
> Of course that wouldn't work with FW we don't have control on. Can Linux
> run on the wii with the original N. FW on the aux. processor ? Can we
> detect what is running there ? Do we care ?
>
Yes, it can. And it is done. But I think we don't care/need that in mainline.
>> It is what Nintendo calls the "Serial Interface" (SI) which is a proprietary
>> serial hardware to drive the gamepads (and a custom keyboard too, once available
>> for an RPG game).
>
> So I would give it a different name than "serial" then. Make it gpsi
> maybe ? (game pad serial interface ?) :-) Or invent something else...
>
I'd choose "gcnsi" with a compatible like "nintendo,gamecube-si" :)
(gcn is the official short abbreviation for the Nintendo GameCube)
> Cheers,
> Ben.
>
Thanks,
Albert
^ permalink raw reply
* Re: [PATCH 04/11] of/flattree: eliminate cell_t typedef
From: Benjamin Herrenschmidt @ 2009-11-26 21:40 UTC (permalink / raw)
To: Segher Boessenkool
Cc: sfr, microblaze-uclinux, devicetree-discuss, sparclinux,
linuxppc-dev, davem
In-Reply-To: <F307E7FB-0819-47E8-9CF4-CA1433C18DA6@kernel.crashing.org>
On Thu, 2009-11-26 at 22:36 +0100, Segher Boessenkool wrote:
> >> You're right, it's not, but makes merging less complex, and then I
> >> can
> >> refactor properly.
> >
> > Still, make them __be32 at least
>
> There is no alignment guarantee at all either, better make it all u8
> and use accessor functions everywhere.
Well... if you want to force using an accessor, then make it an opaque
type. But __be32 is fine. It doesn't necessarily convey alignment and
besides, there happens to -be- aligned in almost all cases so far :-)
The flat tree format guarantees 32-bit alignment for the start of a
property, so we are good here I think.
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 11/19] powerpc: gamecube/wii: flipper interrupt controller support
From: Segher Boessenkool @ 2009-11-26 21:52 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0D65C5.8090304@yahoo.es>
>> Maybe using FLIPPER (or GAMECUBE_FLIPPER) instead of GAMECUBE_COMMON
>> is a good name?
>
> I'd prefer to not use a name that implies a specific hardware to
> describe two (mostly) similar but different hardwares.
Hollywood is 100% compatible to Flipper though.
>>> +/*
>>> + * 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.
>>> + */
I looked it up in YAGCD; it says that _reading_ the ICR reg already
acks all interrupts (and clears the bits), you never write this reg!
>>> +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.
>>
>
> No, it acks just a single IRQ.
No it doesn't. Say IRQs 1 and 3 are asserted, so the reg contains 0x0a.
Now you want to ack IRQ1; set_bit() will write 0x0a | 0x02, not just
0x02.
Segher
^ permalink raw reply
* Re: [RFC PATCH 12/19] powerpc: gamecube: platform support
From: Segher Boessenkool @ 2009-11-26 21:53 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0D6837.4020407@yahoo.es>
> We need it as it currently doesn't match with the default bus ids.
> Should I introduce a .type property matching any of those above in
> the soc node, and get rid of the explicit bus probe?
You don't need any fake bus as far as I can see, just probe the devices
you want.
Segher
^ permalink raw reply
* Re: [RFC PATCH 17/19] powerpc: wii: bootmii starlet 'mini' firmware support
From: Segher Boessenkool @ 2009-11-26 22:00 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0D6DA3.9060906@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.
>
> I didn't know 'replacement' had that semantics.
It's ambiguous.
> My intention was to say that 'mini' firmware is an alternative to
> the stock IOS firmware, not trying to imply that it behaves like it
> or it is compatible with it.
> Point taken.
>
> I'll use 'mini' alternate firmware if that's ok.
"Alternative firmware" is fine.
>>> +enum starlet_ipc_flavour {
>>> + STARLET_IPC_IOS,
>>> + STARLET_IPC_MINI,
>>> +};
>>
>> I thought you don't support IOS at all anymore?
>>
>
> I don't plan to mainline IOS support.
> But to make runtime decisions I need to know if we are running
> along the 'mini' firmware or the default IOS firmware.
> That's the intention of that enum.
>> Modern mini will always have AHBPROT set up to give you full access,
>> so this isn't needed either.
>>
>
> So we have two options here:
> - assume that whatever firmware is running properly sets AHBPROT
> and hope it works
> - or try to use the existing firmware interfaces to check and make
> sure that AHBPROT is indeed properly set
>
> I chose the second option here.
It is much much simpler to simply require a new enough mini :-)
>> Do you need this driver to boot? If not, it might be best to
>> leave it
>> out for now.
>
> Strictly speaking, we don't need the driver to boot if we asume
> that AHBPROT is properly set.
> But we'll need it later to shutdown 'mini' drivers, unless 'mini'
> is changed to boot PowerPC code with all hardware already
> relinquished.
>
> That is not true for the last available version of 'mini' firmware.
> That version still boots PowerPC code with at least the SDHCI
> controller driven by 'mini'.
I'll fix this.
Segher
^ permalink raw reply
* Re: [PATCH] PPC: Sync guest visible MMU state
From: Alexander Graf @ 2009-11-26 21:56 UTC (permalink / raw)
To: Alexander Graf; +Cc: kvm-ppc, kvm@vger.kernel.org, linuxppc-dev@ozlabs.org
In-Reply-To: <1259249056-3953-1-git-send-email-agraf@suse.de>
Am 26.11.2009 um 16:24 schrieb Alexander Graf <agraf@suse.de>:
> Currently userspace has no chance to find out which virtual address
> space we're
> in and resolve addresses. While that is a big problem for migration,
> it's also
> unpleasent when debugging, as gdb and the monitor don't work on
> virtual
> addresses.
>
> This patch exports enough of the MMU segment state to userspace to
> make
> debugging work and thus also includes the groundwork for migration.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
>
> ---
>
> Ben, please take this patch in your tree.
>
> v2 -> v3:
>
> - don't use anonymous unions/structs
> ---
> arch/powerpc/include/asm/kvm.h | 20 +++++++++++-
> arch/powerpc/include/asm/kvm_asm.h | 1 +
> arch/powerpc/include/asm/kvm_book3s.h | 3 ++
> arch/powerpc/kvm/book3s.c | 53 ++++++++++++++++++++++++
> +++++++-
> arch/powerpc/kvm/book3s_64_emulate.c | 38 ++++++++++++++---------
> arch/powerpc/kvm/book3s_64_mmu.c | 2 +
> arch/powerpc/kvm/powerpc.c | 3 ++
> include/linux/kvm.h | 3 ++
> 8 files changed, 104 insertions(+), 19 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/
> asm/kvm.h
> index c9ca97f..cb6ad08 100644
> --- a/arch/powerpc/include/asm/kvm.h
> +++ b/arch/powerpc/include/asm/kvm.h
> @@ -46,8 +46,24 @@ struct kvm_regs {
> };
>
> struct kvm_sregs {
> - __u32 pvr;
> - char pad[1020];
> + union {
> + struct {
> + __u32 pvr;
Meh this breaks old userspace. Better patch following tomorrow.
^ permalink raw reply
* Re: [RFC PATCH 18/19] powerpc: wii: platform support
From: Segher Boessenkool @ 2009-11-26 22:05 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0D7047.3000803@yahoo.es>
>>> +#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.
>
> Yes, at this stage direct hardware should be possible, but only if
> 'mini' support is compiled-in (which will be the default option at
> this stage).
> We can either leave the conditionals as is now, or remove them and
> add them later if we support more than one firmware flavour.
>
> I'm fine with both options.
Removing all superfluous code is best in my opinion.
>>> + 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.
>
> I used an alias because I wanted explicitly the second GPIO word.
>
> Is there another way to select a specific instance of repeated
> identical hardware?
Yes, probe for it with its address. There is no guarantee in which
order
you will get them.
> We have two instances of gpios here, matching the same "compatible".
It's a pain in the behind that the GPIO binding will not allow you to
show the two GPIO regs as one controller, which it is. Or can it
actually do that?
Segher
^ permalink raw reply
* Re: [RFC PATCH 11/19] powerpc: gamecube/wii: flipper interrupt controller support
From: Albert Herranz @ 2009-11-26 22:05 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <7556BF73-5D6D-4817-A02F-3DFA05E9AC58@kernel.crashing.org>
Segher Boessenkool wrote:
>>> Maybe using FLIPPER (or GAMECUBE_FLIPPER) instead of GAMECUBE_COMMON
>>> is a good name?
>>
>> I'd prefer to not use a name that implies a specific hardware to
>> describe two (mostly) similar but different hardwares.
>
> Hollywood is 100% compatible to Flipper though.
>
No. There's no ARAM for example :)
>>>> +/*
>>>> + * 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.
>>>> + */
>
> I looked it up in YAGCD; it says that _reading_ the ICR reg already
> acks all interrupts (and clears the bits), you never write this reg!
>
YAGCD is not always right. You should not take it as _the truth_.
I'm not telling this is the case. But I don't recall such behavior. It's been a long time since this is done this way.
>>>> +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.
>>>
>>
>> No, it acks just a single IRQ.
>
> No it doesn't. Say IRQs 1 and 3 are asserted, so the reg contains 0x0a.
> Now you want to ack IRQ1; set_bit() will write 0x0a | 0x02, not just 0x02.
>
Let me rephrase it. No it should just ack a single IRQ :)
So then this is working by accident.
If that's the case, I guess it works because the interrupt is not cleared at the source and the ICR is set again immediately for any pending interrupt?
>
> Segher
>
>
Thanks,
Albert
^ permalink raw reply
* Re: [RFC PATCH 12/19] powerpc: gamecube: platform support
From: Albert Herranz @ 2009-11-26 22:07 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <76CF44C1-3CC1-43B5-958D-E1A7984A44A9@kernel.crashing.org>
Segher Boessenkool wrote:
>> We need it as it currently doesn't match with the default bus ids.
>
>> Should I introduce a .type property matching any of those above in the
>> soc node, and get rid of the explicit bus probe?
>
> You don't need any fake bus as far as I can see, just probe the devices
> you want.
>
But it's way easier to let the bus probe do it for us. I don't see the win here.
>
> Segher
>
>
Thanks,
Albert
^ permalink raw reply
* Re: [RFC PATCH 02/19] powerpc: gamecube: device tree
From: Segher Boessenkool @ 2009-11-26 22:15 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <1259209436.16367.243.camel@pasglop>
>> If you have only one interrupt controller, like here, you don't
>> need to refer to it _at all_ :-)
>
> I think Linux requires that you do though. It might be a mistake on
> our
> part but heh ...
Linux doesn't require it; (old) Macs are like this, for example,
and that works fine. Oh and all Maple firmwares are like that as
well I think. Etc.
Segher
^ permalink raw reply
* Re: [RFC PATCH 02/19] powerpc: gamecube: device tree
From: Segher Boessenkool @ 2009-11-26 22:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <1259209632.16367.246.camel@pasglop>
>>> + xfb-start = <0x01698000>; /* end-of-ram - xfb-size */
>>> + xfb-size = <0x168000>;
>>
>> XFB address isn't fixed on the hardware, and the kernel might
>> want to move it, and you can easily probe for it anyway. Remove
>> these last two properties please.
>
> Ok but you need to know what it was setup to by the FW no ? To avoid
> having a temporary display "glitch" while booting... Also on a 24M
> config,it might get tough for the driver to allocate 2M contiguous
> like
> that if it's done late in the boot process.
Sure, the platform code will have to probe this very early. Can always
do the /memreserve/ if really needed.
>>> + auxram@0c005000 {
>>> + compatible = "nintendo,flipper-auxram";
>>> + reg = <0x0c005000 0x200>; /* DSP */
>>> + interrupts = <6>;
>>> + interrupt-parent = <&pic>;
>>> + };
>>> +
>>> + audio@0c005000 {
>>> + compatible = "nintendo,flipper-audio";
>>> + reg = <0x0c005000 0x200 /* DSP */
>>> + 0x0c006c00 0x20>; /* AI */
>>> + interrupts = <6>;
>>> + interrupt-parent = <&pic>;
>>> + };
>>
>> These two have the same address, not good. Just remove the
>> auxram node?
>
> Or make it a child of audio ? :-)
Hrm, actually, DSP and AI should be separate nodes, and ARAM a child
of DSP (on its own bus, so it can show that it is 16MB etc.)
Segher
^ permalink raw reply
* Re: [RFC PATCH 03/19] powerpc: gamecube: bootwrapper bits
From: Segher Boessenkool @ 2009-11-26 22:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <1259210142.16367.248.camel@pasglop>
>> So what's your proposal then? Placing it within a fake func?
>
> Just do a .S file :-)
Yeah. You might be able to do one that handles both GC and Wii,
maybe it's easier/clearer to keep them separate though.
> Ouch. I wouldn't be surprised if those guys don't do cache coherency
> in the bridge anyways.
That is correct; we haven't found any way to enable it, at least.
To be fair, it's actually better for performance to _not_ do RAM
coherency when used as a game system.
Segher
^ permalink raw reply
* Re: [RFC PATCH 02/19] powerpc: gamecube: device tree
From: Segher Boessenkool @ 2009-11-26 22:30 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <1259209283.16367.241.camel@pasglop>
>>> + soc {
>>
>> It would be better to rename this as IMMR or the bus type. This node
>> doesn't actually describe the entire chip, but describes the internal
>> memory mapped registers.
>
> I would really just call it "flipper" :-)
Yeah, I came to the same conclusion.
>> Since you're only doing 1:1 mappings; you could replace this with an
>> empty "ranges;" property instead.
>
> On the other hand it is a useful "documentation" to specify the exact
> range decoded when you know it :-)
Not the "decoded" range in this case, that is 0..4G :-) The "canonical"
ranges is nice doc, yes.
>> 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.
>
> Note that this is a linux-ism no ? (aka ePAPRism).
Nope, it is from the standard interrupt mapping recommended practice.
It is fine.
> If they aim toward having a real OF which I think they do
We do.
Segher
^ permalink raw reply
* Re: [RFC PATCH 04/19] powerpc: wii: device tree
From: Benjamin Herrenschmidt @ 2009-11-26 22:37 UTC (permalink / raw)
To: Albert Herranz; +Cc: linuxppc-dev
In-Reply-To: <4B0EF54C.6020101@yahoo.es>
On Thu, 2009-11-26 at 22:38 +0100, Albert Herranz wrote:
> Yup. The idea is to map the first 16MB of MEM1 with a BAT.
> Mapping the whole 24MB with BATs may not be possible because we want the framebuffer
> in MEM1 for performance reasons.
How big is the fb ? We have plenty of BATs on these things... so one 16M
for the low mem and one 64M for the "high" mem, that leaves something
like 6 to manage as much as possible up to the fb :-)
Let's keep 1 or 2 for debug and maybe even BAT map the fb itself, we
still are quite comfortable :-)
That reminds me we still need to re-introduce a clean version of
io_block_mapping (or ioremap flag that does the same) so the platform
code can establish a BAT mapping for a whole bunch of IOs and ioremap
transparenty picks addresses in there. We used to do it, dropped it
mostly because it was done badly (virtual address was hard coded which
caused all sorts of issues) but it shouldn't be hard to do it right and
it would improve perfs a little bit more.
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 04/19] powerpc: wii: device tree
From: Segher Boessenkool @ 2009-11-26 22:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <1259210722.16367.256.camel@pasglop>
>> +/memreserve/ 0x01800000 0xe800000; /* memory hole (includes I/O
>> area) */
>> +/memreserve/ 0x10000000 0x0004000; /* DSP RAM */
>
> Weird layout... nothing you can do about I suppose.
>
> Out of curiosity, what is that DSP RAM ? Some actual DSP core
> somewhere
> in the IO chip setup to use memory from up there ?
It's an actual DSP yes, and it seems it uses that fixed address range
(at the start of the 64MB GDDR3 memory) always. So we have to stay
away from that memory range.
>> + memory {
>> + device_type = "memory";
>> + /* MEM1 + memory hole + MEM2 - firmware/buffers area */
>> + reg = <0x00000000 0x133e0000>;
>> + };
>
> So we do have a nasty hole here in the middle. How bad it is if you
> try
> to just have two ranges in there (ie as if it was discontiguous) ?
Not bad at all. There is no "as if" -- it _is_ discontiguous memory,
they are totally different memory technology even.
>> + /* Team Twiizers' 'mini' firmware IPC */
>
> Out of curiosity, what are these ? :-)
There is an embedded ARM9 in the Hollywood. It is "the boss" of the
system, not the PowerPC. "mini" is the name of the code we run on it.
There is a hardware IPC interface between the ARM and the PowerPC (just
a bunch of doorbells and general purpose regs).
> Similar comment as before, could the above be dynamically probed ? If
> you are not a hacker you may not need that at all to use some linux
> based piece of SW on the wii right ?
Yeah. Well, after any other drivers have been merged, anyway, heh :-)
>> + ohci1@0d060000 {
>
> Why the "1" ?
Right, call them both just "ohci" please. Or "usb" even.
>> + PIC1: pic1@0d800030 {
>> + #interrupt-cells = <1>;
>> + compatible = "nintendo,hollywood-pic";
>> + reg = <0x0d800030 0x8>;
>> + interrupt-controller;
>> + interrupts = <14>;
>> + interrupt-parent = <&PIC0>;
>> + };
>
> Ah, a cascaded PIC, fun fun fun
Well at least this cascaded PIC is sane, the root PIC is less so :-P
>> + hollywood-ahbprot@0d800064 {
>> + compatible = "nintendo,hollywood-ahbprot";
>> + reg = <0x0d800064 0x4>;
>> + };
>
> What is this ?
Most of the devices new in Hollywood (AES engine, NAND engine, USB,
SD, etc.) sit on an AHB bus. The AHBPROT register is used to configure
which of these can be accessed from the PowerPC.
Modern "mini" always gives us full control. Also, there is no reason
to single out the AHBPROT reg in the device tree like this, anyway.
Segher
^ permalink raw reply
* Re: [RFC PATCH 02/19] powerpc: gamecube: device tree
From: Benjamin Herrenschmidt @ 2009-11-26 22:38 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <807D3910-3357-4DBF-A1C7-9C07A7C80A39@kernel.crashing.org>
On Thu, 2009-11-26 at 23:15 +0100, Segher Boessenkool wrote:
> >> If you have only one interrupt controller, like here, you don't
> >> need to refer to it _at all_ :-)
> >
> > I think Linux requires that you do though. It might be a mistake on
> > our
> > part but heh ...
>
> Linux doesn't require it; (old) Macs are like this, for example,
> and that works fine. Oh and all Maple firmwares are like that as
> well I think. Etc.
It works fine on old macs thanks to a quirk we enable in the mac code
iirc and old maple works thanks to mere luck with the fallback code we
have which reads things of PCI config space :-)
Don't rely on that.
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH 04/19] powerpc: wii: device tree
From: Segher Boessenkool @ 2009-11-26 22:50 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <1259211061.16367.260.camel@pasglop>
> BTW. If we want to play with clocks, maybe you should look at
> my proposed binding for clocks and implementing the clk API :-)
There are no clocks that are configurable from the PowerPC side
(well, maybe things like the USB clocks, but we do not know how).
>>> + /* Team Twiizers' 'mini' firmware IPC */
>>> + mini@0d000000 {
>>
>> Although mini is awesome, it isn't hardware, and doesn't
>> belong in the device tree.
>
> So what is at d0000000 ?
0c000000 is the flipper-compatible stuff
0d000000 is the hollywood new devices
0d800000 is the same as part of 0d000000, but with some extra bits
sometimes.
>>> + serial@0d006400 {
>>> + compatible = "nintendo,hollywood-serial";
>>
>> You might want to pick a more descriptive name for this,
>> "serial" is usually understaood to mean "RS232". Which
>> this isn't.
>
> What is it then ? You definitely want some other name if it's not
> classic rs232 style serial.
It's SPI I think, with some extra signals. It's the gamepads mostly.
Segher
^ permalink raw reply
* Re: [RFC PATCH 10/19] powerpc: gamecube/wii: early debugging using usbgecko
From: Segher Boessenkool @ 2009-11-26 22:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Albert Herranz, linuxppc-dev, Arnd Bergmann
In-Reply-To: <1259212468.16367.267.camel@pasglop>
>> This will probably break other platforms if
>> CONFIG_PPC_EARLY_DEBUG_USBGECKO
>> is set. In general, we try hard to make it possible to build generic
>> kernels for multiple systems, so it would be better to also add a
>> runtime check here.
>
> No Arnd. The whole point of EARLY_DEBUG is that it is -known- to break
> other platforms :-) So that we can use it before we even get to probe
> anything.
>
> I am still tempted, one day, to change the kernel early init to
> setup a
> stack in .bss and go straight to C to do that sort of stuff by parsing
> the flat DT, this could be done using pretty much completely generic
> code :-) But not right now.
No Ben. The whole point of EARLY_DEBUG is that it _always works_,
because
it is such trivial code. Don't break that please, or we'll be forced to
add a "REALLY_EARLY_DEBUG" instead :-)
Segher
^ permalink raw reply
* Re: [RFC PATCH 11/19] powerpc: gamecube/wii: flipper interrupt controller support
From: Segher Boessenkool @ 2009-11-26 23:00 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <1259212736.16367.271.camel@pasglop>
>>> +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 */
>>
>> NO_IRQ_IGNORE
>
> Why no just 0 ? (aka NO_IRQ)
>
> Or do you know you are getting lots of spurrious that you don't
> want to
> account ?
IRQ #0 is a valid IRQ here (graphics error IIRC), it should be
remapped I suppose?
Segher
^ permalink raw reply
* Re: [RFC PATCH 16/19] powerpc: wii: hollywood interrupt controller support
From: Segher Boessenkool @ 2009-11-26 23:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <1259213022.16367.273.camel@pasglop>
>> +static void hlwd_pic_mask_and_ack(unsigned int virq)
>> +{
>> + int irq = virq_to_hw(virq);
>> + void __iomem *io_base = get_irq_chip_data(virq);
>> +
>> + clear_bit(irq, io_base + HW_BROADWAY_IMR);
>> + set_bit(irq, io_base + HW_BROADWAY_ICR);
>> +}
>
> Same comment as with flipper. BTW. It looks really similar, can't you
> use the same driver ?
No, they aren't actually similar; the old PIC acks when the status reg
is read, the new PIC acks when you write a 1. Also, the new PIC has
more registers, it is an interrupt controller for the ARM as well. Not
that we use it, but still.
Segher
^ permalink raw reply
* Re: [RFC PATCH 03/19] powerpc: gamecube: bootwrapper bits
From: Segher Boessenkool @ 2009-11-26 23:06 UTC (permalink / raw)
To: Gabriel Paubert; +Cc: Albert Herranz, linuxppc-dev
In-Reply-To: <20091126081724.GA6538@iram.es>
>>> 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?
>>
>> We can make it not do it.
>>
>>> 2) Do lwarx. etc. work in M=0?
>>
>> They should hopefully... as long as you don't rely on the reservation
>> blowing as a result of a DMA write.
>
> Hmm, this really depends on whether the DMA transfers generate bus
> cycles
> that require coherency or not.
They do not; device DMA never goes to the 6xx bus with this bridge.
> Not the other way around. M=1 only forces
> bus cycles to be snooped by other processors (asserting the GBL signal
> on 603/604/750 busses).
Right, it enables sending probes, not receiving them. On this CPU
anyway.
The architecture specification is quite silent on this all.
Segher
^ 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