* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Benjamin Herrenschmidt @ 2006-10-31 4:27 UTC (permalink / raw)
To: Nicolas DET; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <200610292310.k9TNAHXZ013852@post.webmailer.de>
On Mon, 2006-10-30 at 00:10 +0100, Nicolas DET wrote:
> +/*
> + * void call to be used for .ack in the irq_chip_ops
> + * indeed, some of our irq does noy need ack
> + * but the kernel call .ack if it's valid or not
> +*/
> +
> +static void mpc52xx_voidfunc(unsigned int virq)
> +{
> +#ifdef DEBUG
> + int irq;
> + int l2irq;
> +
> + irq = irq_map[virq].hwirq;
> + l2irq = (irq & MPC52xx_IRQ_L2_MASK) >> MPC52xx_IRQ_L2_OFFSET;
> +
> + pr_debug("%s: irq=%x, l2=%d\n", __func__, irq, l2irq);
> +#endif
> +}
As I said on IRC, we might be able to get away without that one, but
that's not urgent.
> + irq = irq_map[virq].hwirq;
> + l2irq = (irq & MPC52xx_IRQ_L2_MASK) >> MPC52xx_IRQ_L2_OFFSET;
> +
> + pr_debug("%s: irq=%x, l2=%d\n", __func__, irq, l2irq);
> +
> + if (l2irq != 0)
> + BUG();
Use BUG_ON(l2irq != 0); instead, generates better code (though I don't
think you really need to keep those checks once you've verified things
work fine).
> + val = in_be32(&intr->ctrl);
> + val &= ~(1 << 11);
> + out_be32(&intr->ctrl, val);
> +}
>From the above, I deduce there is only one possible crit interrupt
right ? Also, it looks a lot, from the rest of the code that it's
basically just "main irq" 0, so why do you do two different L2's for
it ?
In fact, I'm a bit dubious of the way you differenciated "mainirq" and
"main" by giving them the same L2 number while they have different chips
and register sets... that seems to defeat the whole purpose of having
the L2 in the first place don't you think ?
I would have rather an L2 for CRIT + "IRQMAIN" thing (that is interrupts
using intr->ctrl bits 11 and below, I'll let you find a nice "name" for
it, maybe EXTIRQ ? (external irq), then a level for "MAIN" using
intr->main_mask etc...
> + case MPC52xx_IRQ_L1_CRIT:
> + pr_debug("%s: Critical. l2=%x\n", __func__, l2irq);
> +
> + if (l2irq != 0)
> + BUG();
> +
> + type = mpc52xx_irqx_gettype(l2irq);
> + good_irqchip = &mpc52xx_crit_irqchip;
> + break;
> +
> + case MPC52xx_IRQ_L1_MAIN:
> + pr_debug("%s: Main IRQ[1-3] l2=%x\n", __func__, l2irq);
> +
> + if ((l2irq >= 0) && (l2irq <= 3)) {
> + type = mpc52xx_irqx_gettype(l2irq);
> + good_irqchip = &mpc52xx_mainirq_irqchip;
> + } else {
> + good_irqchip = &mpc52xx_main_irqchip;
> + }
> + break;
See my comment above... Also, the only ones that can be edge are the
ones using intr->ctrl, thus if you do things right, you don't need your
fake ack, just only provide an ack for these. For the others, provide an
ack&mask that just masks :) (Though it's debateable wether we should
make the generic ack&mask test for ack beeing NULL indeed, I'll talk to
thomas about it).
> + case MPC52xx_IRQ_L1_PERP:
> + pr_debug("%s: Peripherals. l2=%x\n", __func__, l2irq);
> + good_irqchip = &mpc52xx_periph_irqchip;
> + break;
> +
> + case MPC52xx_IRQ_L1_SDMA:
> + pr_debug("%s: SDMA. l2=%x\n", __func__, l2irq);
> + good_irqchip = &mpc52xx_sdma_irqchip;
> + break;
> +
> + set_irq_chip_and_handler(virq, good_irqchip, good_handle);
> + set_irq_type(virq, type);
set_irq_type() does nothing if you haven't hooked a set_type callback to
your irq_chip and none of yours does so just drop it for now and look at
how this is done in mpic or ipic. If you actually want to implement
proper set_type (which you might need to do if you want that code to
work without having the firmware pre-initialize interrupts with the
right type at boot), you probably want to set the handler in
set_irq_type().
^ permalink raw reply
* Re: [PATCH] qe_lib: Remove immrbar_virt_to_phys() function
From: Kumar Gala @ 2006-10-31 5:05 UTC (permalink / raw)
To: Li Yang-r58472; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <4879B0C6C249214CBE7AB04453F84E4D2632CE@zch01exm20.fsl.freescale.net>
On Oct 30, 2006, at 8:56 PM, Li Yang-r58472 wrote:
>>>> I'm confused, if there are other users of immrbar_virt_to_phy() how
>>>> can you get ride of it completely?
>>>
>>> Other users will change to use iopa() instead. There was some bug
> to
>>> use iopa() with io_block_mapping area, but it is ok to use it now.
>>
>> Hmm, what are the other users doing? I think we should avoid using
>> iopa.
>
> Get physical address from virtual address. I have considered to use
> virt_to_phys(), but it is not consistent between ppc and powerpc arch.
Why does that matter, qe support only exists in powerpc.
- k
^ permalink raw reply
* RE: [PATCH] qe_lib: Remove immrbar_virt_to_phys() function
From: Li Yang-r58472 @ 2006-10-31 5:25 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <F58803B3-7618-4F63-8ED1-B2F72B8F610F@kernel.crashing.org>
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Tuesday, October 31, 2006 1:06 PM
> To: Li Yang-r58472
> Cc: Paul Mackerras; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] qe_lib: Remove immrbar_virt_to_phys() function
>=20
>=20
> On Oct 30, 2006, at 8:56 PM, Li Yang-r58472 wrote:
>=20
> >>>> I'm confused, if there are other users of immrbar_virt_to_phy()
how
> >>>> can you get ride of it completely?
> >>>
> >>> Other users will change to use iopa() instead. There was some bug
> > to
> >>> use iopa() with io_block_mapping area, but it is ok to use it now.
> >>
> >> Hmm, what are the other users doing? I think we should avoid using
> >> iopa.
> >
> > Get physical address from virtual address. I have considered to use
> > virt_to_phys(), but it is not consistent between ppc and powerpc
arch.
>=20
> Why does that matter, qe support only exists in powerpc.
Well, it does use include/asm-ppc/io.h instead of
include/asm-powerpc/io.h for now. If the hack is removed, the driver
will not work.
- Leo
^ permalink raw reply
* Re: [PATCH] qe_lib: Remove immrbar_virt_to_phys() function
From: Kumar Gala @ 2006-10-31 5:30 UTC (permalink / raw)
To: Li Yang-r58472; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <4879B0C6C249214CBE7AB04453F84E4D263314@zch01exm20.fsl.freescale.net>
On Oct 30, 2006, at 11:25 PM, Li Yang-r58472 wrote:
>> -----Original Message-----
>> From: Kumar Gala [mailto:galak@kernel.crashing.org]
>> Sent: Tuesday, October 31, 2006 1:06 PM
>> To: Li Yang-r58472
>> Cc: Paul Mackerras; linuxppc-dev@ozlabs.org
>> Subject: Re: [PATCH] qe_lib: Remove immrbar_virt_to_phys() function
>>
>>
>> On Oct 30, 2006, at 8:56 PM, Li Yang-r58472 wrote:
>>
>>>>>> I'm confused, if there are other users of immrbar_virt_to_phy()
> how
>>>>>> can you get ride of it completely?
>>>>>
>>>>> Other users will change to use iopa() instead. There was some bug
>>> to
>>>>> use iopa() with io_block_mapping area, but it is ok to use it now.
>>>>
>>>> Hmm, what are the other users doing? I think we should avoid using
>>>> iopa.
>>>
>>> Get physical address from virtual address. I have considered to use
>>> virt_to_phys(), but it is not consistent between ppc and powerpc
> arch.
>>
>> Why does that matter, qe support only exists in powerpc.
>
> Well, it does use include/asm-ppc/io.h instead of
> include/asm-powerpc/io.h for now. If the hack is removed, the driver
> will not work.
What uses asm-ppc/io.h instead of asm-powerpc/io.h?
- k
^ permalink raw reply
* RE: [PATCH] qe_lib: Remove immrbar_virt_to_phys() function
From: Li Yang-r58472 @ 2006-10-31 5:34 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <58EAF2FD-1812-45C8-A042-9009EC7BFF49@kernel.crashing.org>
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Tuesday, October 31, 2006 1:30 PM
> To: Li Yang-r58472
> Cc: Paul Mackerras; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] qe_lib: Remove immrbar_virt_to_phys() function
>=20
>=20
> On Oct 30, 2006, at 11:25 PM, Li Yang-r58472 wrote:
>=20
> >> -----Original Message-----
> >> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> >> Sent: Tuesday, October 31, 2006 1:06 PM
> >> To: Li Yang-r58472
> >> Cc: Paul Mackerras; linuxppc-dev@ozlabs.org
> >> Subject: Re: [PATCH] qe_lib: Remove immrbar_virt_to_phys() function
> >>
> >>
> >> On Oct 30, 2006, at 8:56 PM, Li Yang-r58472 wrote:
> >>
> >>>>>> I'm confused, if there are other users of immrbar_virt_to_phy()
> > how
> >>>>>> can you get ride of it completely?
> >>>>>
> >>>>> Other users will change to use iopa() instead. There was some
bug
> >>> to
> >>>>> use iopa() with io_block_mapping area, but it is ok to use it
now.
> >>>>
> >>>> Hmm, what are the other users doing? I think we should avoid
using
> >>>> iopa.
> >>>
> >>> Get physical address from virtual address. I have considered to
use
> >>> virt_to_phys(), but it is not consistent between ppc and powerpc
> > arch.
> >>
> >> Why does that matter, qe support only exists in powerpc.
> >
> > Well, it does use include/asm-ppc/io.h instead of
> > include/asm-powerpc/io.h for now. If the hack is removed, the
driver
> > will not work.
>=20
> What uses asm-ppc/io.h instead of asm-powerpc/io.h?
You can look into the asm-powerpc/io.h. We have hack like this
#ifndef CONFIG_PPC64
#include <asm-ppc/io.h>
#else
........
{whole bunch of defines}
.........
#endif
- Leo
^ permalink raw reply
* Re: glibc-2.5 test suite hangs/crashes the machine
From: Fabio Massimo Di Nitto @ 2006-10-31 6:37 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Jeff Bailey, Paul Mackerras, Ben Collins
In-Reply-To: <1162240610.25682.182.camel@localhost.localdomain>
Benjamin Herrenschmidt wrote:
> On Mon, 2006-10-30 at 12:35 +0100, Fabio Massimo Di Nitto wrote:
>> Benjamin Herrenschmidt wrote:
>>> Does that patch fixes it ?
>>>
>> tested with kernel .17 and headers from .19 and the build hangs. Still tons of
>> Zl processes around.
>>
>> On the note building with kernel .19 and .19 headers it all goes smooth.
>
> Ok, so there's a different issue from what I've found. You haven't by
> chance noted what those processes are (which test case typically) ?
I uploaded a build log and a ps ax output to
http://people.ubuntu.com/~fabbione/benh/
> Also, there's a sysrq to get a backtrace of all pending processes,
> though I don't remember which one off the top of my mind, might be
> useful to have a look though.
I don't remember them either. Anybody?
Fabio
--
I'm going to make him an offer he can't refuse.
^ permalink raw reply
* Re: glibc-2.5 test suite hangs/crashes the machine
From: Fabio Massimo Di Nitto @ 2006-10-31 6:51 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Jeff Bailey, Paul Mackerras, Ben Collins
In-Reply-To: <1162240610.25682.182.camel@localhost.localdomain>
Benjamin Herrenschmidt wrote:
> On Mon, 2006-10-30 at 12:35 +0100, Fabio Massimo Di Nitto wrote:
>> Benjamin Herrenschmidt wrote:
>>> Does that patch fixes it ?
>>>
>> tested with kernel .17 and headers from .19 and the build hangs. Still tons of
>> Zl processes around.
>>
>> On the note building with kernel .19 and .19 headers it all goes smooth.
>
> Ok, so there's a different issue from what I've found. You haven't by
> chance noted what those processes are (which test case typically) ?
> Also, there's a sysrq to get a backtrace of all pending processes,
> though I don't remember which one off the top of my mind, might be
> useful to have a look though.
>
> Ben.
>
Added also dmesg+sysrq.txt at the same url.
Fabio
--
I'm going to make him an offer he can't refuse.
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Sylvain Munaut @ 2006-10-31 6:59 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, linuxppc-embedded, sl, sha
In-Reply-To: <96623161-B847-4F61-94AB-D1F1B1767708@kernel.crashing.org>
Kumar Gala wrote:
>>> * lets drop all the other struct defn in mpc52xx.h. This is a hold
>>> over from arch/ppc and we really should only put defn that we
>>> actually need closer to the code that uses them (ie, drivers, etc.)
>>>
>> Most of the struct that were in mpc52xx.h are the ones used in more than
>> one driver.
>> (or don't really belong to a driver)
>> That's why they've been left there and not in the driver header, so I
>> would leave those
>> there.
>>
>> (e.g. the IDE struct is not there, neither is the FEC. But sdma is used
>> at several place
>> for example, so is rtc, xlb and cdm, ...)
>
> I can see that some of these might be used, like SDMA, XLB, and CDM.
> However, others like RTC should really be done via a single driver.
> Additionally, I'd have to see code use GPIO, SDRAM, GPT, etc before I
> think we should have them defined here. I'd rather we add these items
> as they are used rather than having them here wholesale.
I've just rechecked :
* struct mpc52xx_mmap_ctl;
* struct mpc52xx_sdram;
Not really used any where that I can see/remember. Except for
find_end_of_memory ...
It should however be used in several place in the future ... (sleep support
would need sdram iirc, ...).
But can be removed for now if it's annoying to have them there ...
* struct mpc52xx_intr;
Was used before in platform support code to set the IRQ type of external
IRQ (level/irq) ...
but that can be done with set_irq_type. So can be safetly moved to a
local mpc52xx_pic.h
* struct mpc52xx_rtc;
Was used before in some common code. When the bootloader didn't pass the
bus frequency,
we computed it and the rtc was used to do that. Now, with device tree,
no need for
that anymore. So can be safely removed.
* struct mpc52xx_gpio;
* struct mpc52xx_gpio_wkup;
Port config (pin multiplexing) is in those registers so they should stay
there. This is used
by several driver and platform code. Beside custom driver could use gpio
for different
purpose ...
It could be placed in a include/asm-powerpc/mpc52xx_gpio.h but that
would just make
one more file in include/asm-powerpc so it doesn't make much sens imho.
It should
just stay there.
* struct mpc52xx_xlb;
* struct mpc52xx_cdm;
* struct mpc52xx_sdma;
Used at several place and should really stay there.
Sylvain
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Benjamin Herrenschmidt @ 2006-10-31 7:05 UTC (permalink / raw)
To: Sylvain Munaut; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <4546F46E.5080202@246tNt.com>
On Tue, 2006-10-31 at 07:59 +0100, Sylvain Munaut wrote:
> * struct mpc52xx_mmap_ctl;
> * struct mpc52xx_sdram;
>
> Not really used any where that I can see/remember. Except for
> find_end_of_memory ...
> It should however be used in several place in the future ... (sleep support
> would need sdram iirc, ...).
>
> But can be removed for now if it's annoying to have them there ...
Nah, keep them in. It's not like it was bloating the binary anyway :)
>
> * struct mpc52xx_intr;
>
> Was used before in platform support code to set the IRQ type of external
> IRQ (level/irq) ...
> but that can be done with set_irq_type. So can be safetly moved to a
> local mpc52xx_pic.h
Yup.
> * struct mpc52xx_rtc;
>
> Was used before in some common code. When the bootloader didn't pass the
> bus frequency,
> we computed it and the rtc was used to do that. Now, with device tree,
> no need for
> that anymore. So can be safely removed.
Sounds good.
>
> * struct mpc52xx_gpio;
> * struct mpc52xx_gpio_wkup;
>
> Port config (pin multiplexing) is in those registers so they should stay
> there. This is used
> by several driver and platform code. Beside custom driver could use gpio
> for different
> purpose ...
Yup, though beware of concurrent access to GPIO registers... we might
want a bit of common code with a spinlock in it to "wrap" accesses to
them.
> It could be placed in a include/asm-powerpc/mpc52xx_gpio.h but that
> would just make
> one more file in include/asm-powerpc so it doesn't make much sens imho.
> It should
> just stay there.
Yeah, leave it there.
>
> * struct mpc52xx_xlb;
> * struct mpc52xx_cdm;
> * struct mpc52xx_sdma;
>
> Used at several place and should really stay there.
No need to be too anal about removing things from .h files.
Ben.
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Nicolas DET @ 2006-10-31 7:09 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <1162268830.25682.271.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]
Benjamin Herrenschmidt wrote:
> On Mon, 2006-10-30 at 00:10 +0100, Nicolas DET wrote:
>
>> +/*
>> + * void call to be used for .ack in the irq_chip_ops
>> + * indeed, some of our irq does noy need ack
>> + * but the kernel call .ack if it's valid or not
>> +*/
>> +
>> +static void mpc52xx_voidfunc(unsigned int virq)
>> +{
>> +#ifdef DEBUG
>> + int irq;
>> + int l2irq;
>> +
>> + irq = irq_map[virq].hwirq;
>> + l2irq = (irq & MPC52xx_IRQ_L2_MASK) >> MPC52xx_IRQ_L2_OFFSET;
>> +
>> + pr_debug("%s: irq=%x, l2=%d\n", __func__, irq, l2irq);
>> +#endif
>> +}
>
> As I said on IRC, we might be able to get away without that one, but
> that's not urgent.
Already done.
>
>> + irq = irq_map[virq].hwirq;
>> + l2irq = (irq & MPC52xx_IRQ_L2_MASK) >> MPC52xx_IRQ_L2_OFFSET;
>> +
>> + pr_debug("%s: irq=%x, l2=%d\n", __func__, irq, l2irq);
>> +
>> + if (l2irq != 0)
>> + BUG();
>
> Use BUG_ON(l2irq != 0); instead, generates better code (though I don't
> think you really need to keep those checks once you've verified things
> work fine).
>
Ok.
>> + set_irq_chip_and_handler(virq, good_irqchip, good_handle);
>> + set_irq_type(virq, type);
>
> set_irq_type() does nothing if you haven't hooked a set_type callback to
> your irq_chip and none of yours does so just drop it for now and look at
> how this is done in mpic or ipic. If you actually want to implement
> proper set_type (which you might need to do if you want that code to
> work without having the firmware pre-initialize interrupts with the
> right type at boot), you probably want to set the handler in
> set_irq_type().
>
Our Firmware wil lalways preinit correctly the hw. Moreover, chaning the
setting would propably rpevent PCI IRQ (as PCI are IRQ[0-3].
So I should just remove the set_irq_type() call?
Regards
[-- Attachment #2: nd.vcf --]
[-- Type: text/x-vcard, Size: 249 bytes --]
begin:vcard
fn:Nicolas DET ( bplan GmbH )
n:DET;Nicolas
org:bplan GmbH
adr:;;;;;;Germany
email;internet:nd@bplan-gmbh.de
title:Software Entwicklung
tel;work:+49 6171 9187 - 31
x-mozilla-html:FALSE
url:http://www.bplan-gmbh.de
version:2.1
end:vcard
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Nicolas DET @ 2006-10-31 7:10 UTC (permalink / raw)
To: Dale Farnsworth; +Cc: linuxppc-embedded
In-Reply-To: <20061030174724.7155.qmail@farnsworth.org>
[-- Attachment #1: Type: text/plain, Size: 459 bytes --]
Dale Farnsworth wrote:
> In article <200610292310.k9TNAHXZ013852@post.webmailer.de> Nicolas wrote:
>> This patch add MPC52xx Interrupt controller for ARCH=powerpc.
>>
>> It includes the main code in arch/powerpc/sysdev/ ad well as an header file in
>> include/asm-powerpc.
>>
>> Signed-off-by: Nicolas DET <nd@bplan-gmbh.de>
>
> I'd like to see us go the other direction, with something like the
> following (untested code).
>
I'll include this and test.
[-- Attachment #2: nd.vcf --]
[-- Type: text/x-vcard, Size: 249 bytes --]
begin:vcard
fn:Nicolas DET ( bplan GmbH )
n:DET;Nicolas
org:bplan GmbH
adr:;;;;;;Germany
email;internet:nd@bplan-gmbh.de
title:Software Entwicklung
tel;work:+49 6171 9187 - 31
x-mozilla-html:FALSE
url:http://www.bplan-gmbh.de
version:2.1
end:vcard
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Nicolas DET @ 2006-10-31 7:14 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <7BDB728E-0CC2-4940-9856-B496022F3482@kernel.crashing.org>
[-- Attachment #1: Type: text/plain, Size: 1457 bytes --]
Kumar Gala wrote:
>
> On Oct 29, 2006, at 5:10 PM, Nicolas DET wrote:
>
>> This patch add MPC52xx Interrupt controller for ARCH=powerpc.
>>
>> It includes the main code in arch/powerpc/sysdev/ ad well as an header
>> file in
>> include/asm-powerpc.
>>
>> Signed-off-by: Nicolas DET <nd@bplan-gmbh.de>
>
> Can you see if you can figure out how to inline patches with your
> mailer, its really difficult to comment on issues w/an attachment.
>
OT:
Well, on a personal point of view, the only usable mailer I know does
not run on my 'work' OS. I'll copy/paste from an editor ;-)
> +/* MBAR position */
> +#define MPC52xx_MBAR 0xf0000000 /* Phys address */
> +#define MPC52xx_MBAR_VIRT 0xf0000000 /* Virt address */
> +#define MPC52xx_MBAR_SIZE 0x00010000
> +
> +#define MPC52xx_PA(x) ((phys_addr_t)(MPC52xx_MBAR + (x)))
> +#define MPC52xx_VA(x) ((void __iomem *)(MPC52xx_MBAR_VIRT + (x)))
>
> This should be handled dynamically (pulled from the device tree), I
> doubt MBAR will be at the same location for all boards.
Well, 0xf000000 seems some kind of 'standart' value. we could have a
global variable 'mpc52xx_mbar' which would be default 0xf0000000 and
modified by each platform.
> * can you split out the interrupt controller header info into a
> mpc52xx_pic.h [mpc52xx_intr, MPC52xx_IRQ_...]
Well, I dod personally have the whole structure/define in a single
header rather than splitting in xxx files.
[-- Attachment #2: nd.vcf --]
[-- Type: text/x-vcard, Size: 249 bytes --]
begin:vcard
fn:Nicolas DET ( bplan GmbH )
n:DET;Nicolas
org:bplan GmbH
adr:;;;;;;Germany
email;internet:nd@bplan-gmbh.de
title:Software Entwicklung
tel;work:+49 6171 9187 - 31
x-mozilla-html:FALSE
url:http://www.bplan-gmbh.de
version:2.1
end:vcard
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Benjamin Herrenschmidt @ 2006-10-31 7:21 UTC (permalink / raw)
To: Nicolas DET; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <4546F68C.6070900@bplan-gmbh.de>
> > set_irq_type() does nothing if you haven't hooked a set_type callback to
> > your irq_chip and none of yours does so just drop it for now and look at
> > how this is done in mpic or ipic. If you actually want to implement
> > proper set_type (which you might need to do if you want that code to
> > work without having the firmware pre-initialize interrupts with the
> > right type at boot), you probably want to set the handler in
> > set_irq_type().
> >
>
> Our Firmware wil lalways preinit correctly the hw. Moreover, chaning the
> setting would propably rpevent PCI IRQ (as PCI are IRQ[0-3].
>
> So I should just remove the set_irq_type() call?
Just remove it for now. I'll look into making things better later. Your
firmware might setup the stuff properly, but others will probably not
and this driver will be used by others :)
Ben.
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Benjamin Herrenschmidt @ 2006-10-31 7:38 UTC (permalink / raw)
To: Nicolas DET; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <4546F7DE.6070104@bplan-gmbh.de>
> Well, on a personal point of view, the only usable mailer I know does
> not run on my 'work' OS. I'll copy/paste from an editor ;-)
And both are ? (mailer and 'work' OS ? :)
> > +/* MBAR position */
> > +#define MPC52xx_MBAR 0xf0000000 /* Phys address */
> > +#define MPC52xx_MBAR_VIRT 0xf0000000 /* Virt address */
> > +#define MPC52xx_MBAR_SIZE 0x00010000
> > +
> > +#define MPC52xx_PA(x) ((phys_addr_t)(MPC52xx_MBAR + (x)))
> > +#define MPC52xx_VA(x) ((void __iomem *)(MPC52xx_MBAR_VIRT + (x)))
> >
> > This should be handled dynamically (pulled from the device tree), I
> > doubt MBAR will be at the same location for all boards.
>
> Well, 0xf000000 seems some kind of 'standart' value. we could have a
> global variable 'mpc52xx_mbar' which would be default 0xf0000000 and
> modified by each platform.
No. No magic globals. If we need some common code for dealing with some
52xx specific bits, them have a file somewhere, possibly in sysdev,
containing those and exposing functions.
> > * can you split out the interrupt controller header info into a
> > mpc52xx_pic.h [mpc52xx_intr, MPC52xx_IRQ_...]
>
> Well, I dod personally have the whole structure/define in a single
> header rather than splitting in xxx files.
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Nicolas DET @ 2006-10-31 7:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <1162279296.25682.297.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 996 bytes --]
Benjamin Herrenschmidt wrote:
>>> set_irq_type() does nothing if you haven't hooked a set_type callback to
>>> your irq_chip and none of yours does so just drop it for now and look at
>>> how this is done in mpic or ipic. If you actually want to implement
>>> proper set_type (which you might need to do if you want that code to
>>> work without having the firmware pre-initialize interrupts with the
>>> right type at boot), you probably want to set the handler in
>>> set_irq_type().
>>>
>> Our Firmware wil lalways preinit correctly the hw. Moreover, chaning the
>> setting would propably rpevent PCI IRQ (as PCI are IRQ[0-3].
>>
>> So I should just remove the set_irq_type() call?
>
> Just remove it for now. I'll look into making things better later. Your
> firmware might setup the stuff properly,
Removed
>but others will probably not
> and this driver will be used by others :)
>
In this case: shouldn't the platform init correctly setup the hw instead
of the Firmware?
Regards,
[-- Attachment #2: nd.vcf --]
[-- Type: text/x-vcard, Size: 249 bytes --]
begin:vcard
fn:Nicolas DET ( bplan GmbH )
n:DET;Nicolas
org:bplan GmbH
adr:;;;;;;Germany
email;internet:nd@bplan-gmbh.de
title:Software Entwicklung
tel;work:+49 6171 9187 - 31
x-mozilla-html:FALSE
url:http://www.bplan-gmbh.de
version:2.1
end:vcard
^ permalink raw reply
* Re: Xubuntu and Xenomai on the Lite5200 (IceCube)...
From: David Jander @ 2006-10-31 7:54 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <528646bc0610300958p627a79abyd84059854a8ca47@mail.gmail.com>
On Monday 30 October 2006 18:58, Grant Likely wrote:
>[...]
>>
> Congratulations, that's good work! Especially the bit about
> documenting what you did! I've been meaning to play w/ xenomai for a
> while now.
Thanks. I hope this makes searching-time shorter for you.
I wish I could remember more details about what I exactly changed to make the
kernel compile after backporting bestcomm and FEC... I vaguely remember
adding "#include <platform_device.h>" to at least two files...
> Care to help with the port to arch/powerpc now? :P
I would really like to, but I have very limited time to dedicate to such
things. If I get the chance to do something about it, I'll do it, I promise.
The time will come when I'll have to port our own board-support stuff to
arch/powerpc... so...
Greetings,
--
David Jander
^ permalink raw reply
* [PATCH] CPM UART: Fix break generation
From: Laurent Pinchart @ 2006-10-31 7:58 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linuxppc-embedded
In-Reply-To: <20061030184002.35611755@vitb.ru.mvista.com>
When generating a break condition on a serial port, the CPM must be told
beforehand how long the break should be. Unfortunately, this information is
not available through the current serial break handling API. This patch works
around the problem by requesting a 32767 characters break followed by a 0
characters break after the requested duration. The CPM will stop the first
break when the second one is requested. This might not work with future CPM
revisions.
Signed-off-by: Laurent Pinchart <laurent@pclaurent.technotrade>
---
drivers/serial/cpm_uart/cpm_uart_core.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c
b/drivers/serial/cpm_uart/cpm_uart_core.c
index c29d87d..aa0a284 100644
--- a/drivers/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/serial/cpm_uart/cpm_uart_core.c
@@ -239,14 +239,23 @@ static void cpm_uart_break_ctl(struct ua
{
struct uart_cpm_port *pinfo = (struct uart_cpm_port *)port;
int line = pinfo - cpm_uart_ports;
+ volatile u16 *brkcr = IS_SMC(pinfo) ? &pinfo->smcup->smc_brkcr
+ : &pinfo->sccup->scc_brkcr;
pr_debug("CPM uart[%d]:break ctrl, break_state: %d\n", port->line,
break_state);
if (break_state)
+ {
+ *brkcr = 32767;
cpm_line_cr_cmd(line, CPM_CR_STOP_TX);
+ }
else
+ {
+ *brkcr = 0;
+ cpm_line_cr_cmd(line, CPM_CR_STOP_TX);
cpm_line_cr_cmd(line, CPM_CR_RESTART_TX);
+ }
}
/*
--
1.4.GIT
^ permalink raw reply related
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Benjamin Herrenschmidt @ 2006-10-31 7:58 UTC (permalink / raw)
To: Nicolas DET; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <45470006.1020004@bplan-gmbh.de>
On Tue, 2006-10-31 at 08:49 +0100, Nicolas DET wrote:
> In this case: shouldn't the platform init correctly setup the hw instead
> of the Firmware?
No. The device-tree contains the polarity information, it gets passed to
the PIC driver via set_irq_type() anyway so there is no need in theory
to have the HW initialized by the firmware if that is implemented. In
the case where it's not, however, you indeed need to make sure the
firmware have programmed the same settings in the HW as are exposed in
the device-tree.
Ben.
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Nicolas DET @ 2006-10-31 8:25 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <1162280335.25682.302.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 1970 bytes --]
Benjamin Herrenschmidt wrote:
>> Well, on a personal point of view, the only usable mailer I know does
>> not run on my 'work' OS. I'll copy/paste from an editor ;-)
>
> And both are ? (mailer and 'work' OS ? :)
>
Mailer: SimpleMail. Almost usable a bit buggy.
'work' OS: Linux PowerPC ;-)
>>> +/* MBAR position */
>>> +#define MPC52xx_MBAR 0xf0000000 /* Phys address */
>>> +#define MPC52xx_MBAR_VIRT 0xf0000000 /* Virt address */
>>> +#define MPC52xx_MBAR_SIZE 0x00010000
>>> +
>>> +#define MPC52xx_PA(x) ((phys_addr_t)(MPC52xx_MBAR + (x)))
>>> +#define MPC52xx_VA(x) ((void __iomem *)(MPC52xx_MBAR_VIRT + (x)))
>>>
>>> This should be handled dynamically (pulled from the device tree), I
>>> doubt MBAR will be at the same location for all boards.
>> Well, 0xf000000 seems some kind of 'standart' value. we could have a
>> global variable 'mpc52xx_mbar' which would be default 0xf0000000 and
>> modified by each platform.
>
> No. No magic globals. If we need some common code for dealing with some
> 52xx specific bits, them have a file somewhere, possibly in sysdev,
> containing those and exposing functions.
>
Ok. By the way, the mbar is include as property in our OpenFrimware.
Moreover, the G2CORE CPU has a new SPR 'MBAR' which is the MBAR ;-). It
would maybe make sense to create a new file
(arch/powerpc/sysdev/mpc52xx.c) which would contain chip specific code.
Like mpc52xx_get_mbar(), mpc52xx_get_ipbfreq(), etc...
I updated a bit the patches. I applied Dale requests.
My kernel still compiles and boots. ;-)
Should I post the new patches?
About the headers thingy. Should I split them directly in the patch, or
this should be done by others later on?
About the serials and USB part. Have anyone already do some work (moving
to of_platform, etc...)?
As far as I see, the Efika platform patch did not get any comment,
should I assume it is good enough for inclusion in the main stream kernel?
Regards.
[-- Attachment #2: nd.vcf --]
[-- Type: text/x-vcard, Size: 249 bytes --]
begin:vcard
fn:Nicolas DET ( bplan GmbH )
n:DET;Nicolas
org:bplan GmbH
adr:;;;;;;Germany
email;internet:nd@bplan-gmbh.de
title:Software Entwicklung
tel;work:+49 6171 9187 - 31
x-mozilla-html:FALSE
url:http://www.bplan-gmbh.de
version:2.1
end:vcard
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Nicolas DET @ 2006-10-31 8:28 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <1162281518.25682.305.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 794 bytes --]
Benjamin Herrenschmidt wrote:
> On Tue, 2006-10-31 at 08:49 +0100, Nicolas DET wrote:
>
>> In this case: shouldn't the platform init correctly setup the hw instead
>> of the Firmware?
>
> No. The device-tree contains the polarity information, it gets passed to
> the PIC driver via set_irq_type() anyway so there is no need in theory
> to have the HW initialized by the firmware if that is implemented. In
> the case where it's not, however, you indeed need to make sure the
> firmware have programmed the same settings in the HW as are exposed in
> the device-tree.
>
In my point of view, the Firmware main task is to init the HW. I do not
think it's a good idea to overwrite firmware hw settings in the common
code. In my mind, this should be done in platform specific area.
Regards,
[-- Attachment #2: nd.vcf --]
[-- Type: text/x-vcard, Size: 249 bytes --]
begin:vcard
fn:Nicolas DET ( bplan GmbH )
n:DET;Nicolas
org:bplan GmbH
adr:;;;;;;Germany
email;internet:nd@bplan-gmbh.de
title:Software Entwicklung
tel;work:+49 6171 9187 - 31
x-mozilla-html:FALSE
url:http://www.bplan-gmbh.de
version:2.1
end:vcard
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Benjamin Herrenschmidt @ 2006-10-31 8:42 UTC (permalink / raw)
To: Nicolas DET; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <4547086D.2050808@bplan-gmbh.de>
On Tue, 2006-10-31 at 09:25 +0100, Nicolas DET wrote:
> Ok. By the way, the mbar is include as property in our OpenFrimware.
> Moreover, the G2CORE CPU has a new SPR 'MBAR' which is the MBAR ;-). It
> would maybe make sense to create a new file
> (arch/powerpc/sysdev/mpc52xx.c) which would contain chip specific code.
>
> Like mpc52xx_get_mbar(), mpc52xx_get_ipbfreq(), etc...
>
> I updated a bit the patches. I applied Dale requests.
> My kernel still compiles and boots. ;-)
>
> Should I post the new patches?
Sure.
> About the headers thingy. Should I split them directly in the patch, or
> this should be done by others later on?
Don't bother that much about splitting the headers. I personally don't
mind.
> About the serials and USB part. Have anyone already do some work (moving
> to of_platform, etc...)?
I think Grant has.
> As far as I see, the Efika platform patch did not get any comment,
> should I assume it is good enough for inclusion in the main stream kernel?
It looked ok at a very very quick glance. I'll give a better look.
Ben.
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Benjamin Herrenschmidt @ 2006-10-31 8:44 UTC (permalink / raw)
To: Nicolas DET; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <4547093E.6070502@bplan-gmbh.de>
> In my point of view, the Firmware main task is to init the HW. I do not
> think it's a good idea to overwrite firmware hw settings in the common
> code. In my mind, this should be done in platform specific area.
The setting of interrupt polarity is a matter that is at the edge
between firmware and OS responsibility. Remember that a lot of embedded
platforms have close to no useful firmware too.
There are reasons why there is an API for those. As long as the
device-tree contains correct polarity, it should be fine to let the pic
driver to the job as told.
Of course, if your device-tree has bugs, then adding that feature
afterward will suddenly break efika ...
Ben.
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Nicolas DET @ 2006-10-31 9:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <1162284291.25682.323.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 710 bytes --]
Benjamin Herrenschmidt wrote:
>> In my point of view, the Firmware main task is to init the HW. I do not
>> think it's a good idea to overwrite firmware hw settings in the common
>> code. In my mind, this should be done in platform specific area.
>
> The setting of interrupt polarity is a matter that is at the edge
> between firmware and OS responsibility. Remember that a lot of embedded
> platforms have close to no useful firmware too.
>
Ok. that's why I suggest to keep buggy (or none) firmware board in
platform specific code.
> Of course, if your device-tree has bugs, then adding that feature
> afterward will suddenly break efika ...
>
Our OpenFirmware is, of course, bugfree ;-)
Regards,
[-- Attachment #2: nd.vcf --]
[-- Type: text/x-vcard, Size: 249 bytes --]
begin:vcard
fn:Nicolas DET ( bplan GmbH )
n:DET;Nicolas
org:bplan GmbH
adr:;;;;;;Germany
email;internet:nd@bplan-gmbh.de
title:Software Entwicklung
tel;work:+49 6171 9187 - 31
x-mozilla-html:FALSE
url:http://www.bplan-gmbh.de
version:2.1
end:vcard
^ permalink raw reply
* Re: [PATCH] CPM UART: Fix break generation
From: Pantelis Antoniou @ 2006-10-31 9:04 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linuxppc-embedded
In-Reply-To: <200610310858.28622.laurent.pinchart@tbox.biz>
On 31 =CE=9F=CE=BA=CF=84 2006, at 9:58 =CE=A0=CE=9C, Laurent Pinchart =
wrote:
> When generating a break condition on a serial port, the CPM must be =20=
> told
> beforehand how long the break should be. Unfortunately, this =20
> information is
> not available through the current serial break handling API. This =20
> patch works
> around the problem by requesting a 32767 characters break followed =20
> by a 0
> characters break after the requested duration. The CPM will stop =20
> the first
> break when the second one is requested. This might not work with =20
> future CPM
> revisions.
>
ACK,
Nice one Laurent ;)
Regards
Pantelis
^ permalink raw reply
* Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc
From: Benjamin Herrenschmidt @ 2006-10-31 9:07 UTC (permalink / raw)
To: Nicolas DET; +Cc: linuxppc-dev, sl, sha, linuxppc-embedded
In-Reply-To: <4547119C.3060208@bplan-gmbh.de>
On Tue, 2006-10-31 at 10:04 +0100, Nicolas DET wrote:
> Benjamin Herrenschmidt wrote:
> >> In my point of view, the Firmware main task is to init the HW. I do not
> >> think it's a good idea to overwrite firmware hw settings in the common
> >> code. In my mind, this should be done in platform specific area.
> >
> > The setting of interrupt polarity is a matter that is at the edge
> > between firmware and OS responsibility. Remember that a lot of embedded
> > platforms have close to no useful firmware too.
> >
>
> Ok. that's why I suggest to keep buggy (or none) firmware board in
> platform specific code.
Well, in that case, we have a well defined interface to set the sense
code, via the device-tree, and that's much better than having platform
code muck around the PIC hardware separately from the PIC driver don't
you think ?
Anyway, that's the way it works in Linux/powerpc so there is no need
debating that for ever. Just be aware that at one point, there will be a
set_type() implementation in this driver and that it will be called
based on the polarity information in the device-tree so make sure you
got it right.
> > Of course, if your device-tree has bugs, then adding that feature
> > afterward will suddenly break efika ...
> >
>
> Our OpenFirmware is, of course, bugfree ;-)
:)
Ben.
^ 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