* Re: [PATCH] Fix the LPC47M192 SuperIO on the MPC8641 HPCN
From: Segher Boessenkool @ 2007-06-08 8:22 UTC (permalink / raw)
To: Andy Fleming; +Cc: linuxppc-dev, paulus, Arnd Bergmann
In-Reply-To: <FF15186D-1D92-422E-AC6C-3127F6116C79@freescale.com>
>>> I suppose I could create a device node for the Super I/O config
>>> registers and use those instead of hardcoding it here.
>>
>> I'd just hide it all, do this setup in the firmware,
>> where it belongs, and don't expose the superio config
>> in the device tree.
>
> No more. No more firmware-only initializations.
This setup is very board specific, and the board cannot
reasonably work without that setup being done right. You
really want to push _that_ into Linux? Alternatively,
you could put it into the device tree, but that doesn't
help anything either.
> It sounds great, in principle, until you actually have to figure out
> why someone's setup isn't working. I'm tired of having to see if the
> dts, u-boot, and Linux are in sync. If Linux wants to use a device, I
> think it's not unreasonable to have it setup the device itself. That
> way, Linux can do whatever it wants with the device, and not have to
> rely on U-Boot (or some other firmware) setting up the appropriate
> bits.
There is one and only one way to set up the superio for
a certain board (assuming the legacy I/O and IRQ values
are considered fixed values).
> Please...no. What happens next is that we find a small bug that
> requires we modify U-Boot to do the initialization slightly
> differently, and then requires Linux to act slightly differently.
This is equivalent to needing a board-level fix really.
Segher
^ permalink raw reply
* Re: [patch 05/18] PS3: Fix sparse warnings
From: Takao Shinohara @ 2007-06-08 5:59 UTC (permalink / raw)
To: Geoff Levand
Cc: Geert Uytterhoeven, Noguchi, Masato, Paul Mackerras,
Arnd Bergmann, linuxppc-dev
In-Reply-To: <4668176D.7060002@am.sony.com>
[-- Attachment #1: Type: text/plain, Size: 2450 bytes --]
On 2007/06/07, at 23:34, Geoff Levand wrote:
> Arnd Bergmann wrote:
>> On Wednesday 06 June 2007, Geoff Levand wrote:
>>> -╴╴╴╴╴╴╴spu->local_store = ioremap(spu->local_store_phys, LS_SIZE);
>>> +╴╴╴╴╴╴╴spu->local_store = (__force void
>>> *)ioremap(spu->local_store_phys,
>>> +╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴╴ ╴ LS_SIZE);
>>
>> I haven't noticed this before, but it seems to be a preexisting bug:
>> You map the local_store as with the guarded page table bit set, which
>> causes a performance degradation when accessing the memory from kernel
>> space.
>>
>> If you're lucky, your hypervisor knows this and will fix it up for
>> you, but I would replace the ioremap call with an
>> ioremap_flags(..., _PAGE_NO_CACHE); to be on the safe side.
>>
>> If you want to measure the impact, I'd suggest timing a user space
>> read() on the mem file of a running SPU context.
>
> Hi Arnd,
>
> I asked Noguchi-san to check the performance and below is his
> report and test program. I'll add the change into my patch set.
>
> -Geoff
>
> -------- Original Message --------
> Subject: RE: [patch 05/18] PS3: Fix sparse warnings
> Date: Thu, 7 Jun 2007 05:39:43 -0700
> From: Noguchi, Masato <Masato.Noguchi@jp.sony.com>
> To: Levand, Geoff <Geoffrey.Levand@am.sony.com>
>
> << A time to read a whole of LS by read system call >>
> not patched: avg. 21053.7800 tick ( 263.831830 microseconds )
> patched: avg. 20809.2412 tick ( 260.767434 microseconds )
>
> about 1% faster.
> I think it's a valid difference. (not a measurement error.)
Let me correct above measurements and analysis.
My understanding is:
1) caching-inhibited loads are usually implemented as guarded. Thus,
guarded property will not affect load performance.
2) caching-inhibited and non-guarded sequential stores are usually
gathered (merged) in store buffer. Thus, guarded property will
affect store performance much.
Therefor, we should measure write(2) performance to evaluate the impact
of the patch.
Attached is a little modified test program based on Noguchi-san's one.
It measures write(2) performance. Buffer area is 'pre-touched' to avoid
VM overhead.
***** test results *****
before: 103903 [TB cycles] (best value out of 100 tries.)
after: 14277 [TB cycles] (best value out of 100 tries.)
-- Takao Shinohara
[-- Attachment #2: test_write_ls.c --]
[-- Type: text/plain, Size: 1482 bytes --]
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/mman.h>
#include <sys/wait.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdio.h>
#include <signal.h>
#include <string.h>
#include <stdint.h>
#include <stdlib.h>
#include <pthread.h>
#define __NR_spe_run 278
#define __NR_spe_create 279
#define LS_SIZE 0x40000
#define SPENODE "/spu/stoplooptest"
#define MFTB(RA) __asm__ volatile("mftb %0":"=r"(RA))
long long do_test(void)
{
int spefd = -1, lsfd = -1;
int npc, status;
long long ret = -1;
char buf[LS_SIZE];
int n;
uint32_t t1, t2;
/* create context */
spefd = syscall(__NR_spe_create, SPENODE, 0,
S_IRUSR | S_IWUSR | S_IXUSR);
if (spefd < 0) goto out;
/* run once to assign physical spe */
npc = 0;
syscall(__NR_spe_run, spefd, &npc, &status);
/* get /mem file descriptor */
lsfd = open(SPENODE "/mem", O_RDWR,
S_IRUSR | S_IWUSR);
if (lsfd < 0) goto out;
/* make sure main memory is allocated and writable for 'buf' */
memset(buf, 0, sizeof buf);
/* write mem */
MFTB(t1);
if (write(lsfd, buf, LS_SIZE) != LS_SIZE) {
goto out;
}
MFTB(t2);
ret = t2 - t1;
out:
if ( lsfd >= 0 ) close(lsfd);
if ( spefd >= 0 ) close(spefd);
return ret;
}
int main(int argc, char *argv[])
{
long long r;
r = do_test();
printf("%lld\n", r);
return 0;
}
[-- Attachment #3: Type: text/plain, Size: 2 bytes --]
^ permalink raw reply
* [PATCH] do firmware feature fixups after features are initialised
From: Michael Neuling @ 2007-06-08 4:00 UTC (permalink / raw)
To: paulus; +Cc: linuxppc-dev
On pSeries the firmware features are not setup until ppc_md.init_early,
so we can't do the firmware feature sections fixups till after this.
Currently firmware feature sections is only used on iSeries which inits
the firmware features much earlier. This is a bug in waiting on
pSeries.
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
paulus: since we aren't hitting this currently, it can wait for 2.6.23.
arch/powerpc/kernel/setup_64.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
Index: linux-2.6-ozlabs/arch/powerpc/kernel/setup_64.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/setup_64.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/setup_64.c
@@ -350,13 +350,11 @@ void __init setup_system(void)
{
DBG(" -> setup_system()\n");
- /* Apply the CPUs-specific and firmware specific fixups to kernel
- * text (nop out sections not relevant to this CPU or this firmware)
+ /* Apply CPUs-specific fixups to kernel text (nop out sections
+ * not relevant to this CPU)
*/
do_feature_fixups(cur_cpu_spec->cpu_features,
&__start___ftr_fixup, &__stop___ftr_fixup);
- do_feature_fixups(powerpc_firmware_features,
- &__start___fw_ftr_fixup, &__stop___fw_ftr_fixup);
/*
* Unflatten the device-tree passed by prom_init or kexec
@@ -394,6 +392,12 @@ void __init setup_system(void)
if (ppc_md.init_early)
ppc_md.init_early();
+ /* Apply firmware specific fixups to kernel text (nop out
+ * sections not relevant to this firmware)
+ */
+ do_feature_fixups(powerpc_firmware_features,
+ &__start___fw_ftr_fixup, &__stop___fw_ftr_fixup);
+
/*
* We can discover serial ports now since the above did setup the
* hash table management for us, thus ioremap works. We do that early
^ permalink raw reply
* [PATCH] fix stolen time for SMT without LPAR
From: Michael Neuling @ 2007-06-08 3:18 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <18023.24703.555674.28511@cargo.ozlabs.ibm.com>
For POWERPC, stolen time accounts for cycles lost to the hypervisor or
PURR cycles attributed to the other SMT thread. Hence, when a PURR is
available, we should still calculate stolen time, irrespective of being
virtualised.
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
> This means that on a machine with SPLPAR but not a PURR, e.g. a JS21,
> we'll try to read the PURR register and do the stolen time calculation
> with whatever value was in the register before the mfspr from the
> PURR. If we're going to make this change then we also need to add
> code to do the H_PURR hcall instead of reading the PURR directly on
> such machines.
This version bails on reading the PURR if it doesn't exist, so it
doesn't make JS21 any worse. We still need to fix JS21, but our initial
patches using H_PURR showed some lmbench crappy-ness.
Tested on JS21 and BML. Like you said offline, this can be 2.6.23.
arch/powerpc/kernel/time.c | 2 +-
include/asm-powerpc/time.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Index: linux-2.6-ozlabs/arch/powerpc/kernel/time.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/time.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/time.c
@@ -214,7 +214,6 @@ static void account_process_time(struct
run_posix_cpu_timers(current);
}
-#ifdef CONFIG_PPC_SPLPAR
/*
* Stuff for accounting stolen time.
*/
@@ -273,6 +272,7 @@ void calculate_steal_time(void)
spin_unlock(&pme->lock);
}
+#ifdef CONFIG_PPC_SPLPAR
/*
* Must be called before the cpu is added to the online map when
* a cpu is being brought up at runtime.
Index: linux-2.6-ozlabs/include/asm-powerpc/time.h
===================================================================
--- linux-2.6-ozlabs.orig/include/asm-powerpc/time.h
+++ linux-2.6-ozlabs/include/asm-powerpc/time.h
@@ -232,7 +232,7 @@ extern void account_process_vtime(struct
#define account_process_vtime(tsk) do { } while (0)
#endif
-#if defined(CONFIG_VIRT_CPU_ACCOUNTING) && defined(CONFIG_PPC_SPLPAR)
+#if defined(CONFIG_VIRT_CPU_ACCOUNTING)
extern void calculate_steal_time(void);
extern void snapshot_timebases(void);
#else
^ permalink raw reply
* Re: [RFC/PATCH 4/4] Add support for MSI on Axon-based Cell systems
From: Michael Ellerman @ 2007-06-08 2:53 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200706041912.04651.arnd@arndb.de>
[-- Attachment #1: Type: text/plain, Size: 958 bytes --]
On Mon, 2007-06-04 at 19:12 +0200, Arnd Bergmann wrote:
> On Monday 04 June 2007, Michael Ellerman wrote:
> > + for_each_compatible_node(node, NULL, "ibm,axon-msic") {
> > + if (axon_msi_setup_one(of_node_get(node)) == 0)
> > + found++;
> > + }
> > + of_node_put(node);
>
> One more thing: AFAICS 'node' is guaranteed to be NULL when you get
> out of the for_each_compatible_node loop, so you don't need the
> of_node_put().
Yeah it is. I kind of like having the put there, because the idiom is
often to have a for_each_* with a break in it, in which case you need
the put.
But in this case it's a waste so I'll remove it.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: Add Marvell mv64x60 udbg putc/getc functions
From: Michael Ellerman @ 2007-06-08 2:15 UTC (permalink / raw)
To: Dale Farnsworth; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20070606172900.GA29951@xyzzy.farnsworth.org>
[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]
On Wed, 2007-06-06 at 10:29 -0700, Dale Farnsworth wrote:
> On Wed, Jun 06, 2007 at 04:06:05PM +1000, Paul Mackerras wrote:
> > Dale Farnsworth writes:
> >
> > > Paul, this patch is unchanged from the one I posted 10 days ago.
> > > At that time, I saw no discussion, other than Mark's ACK. I'd
> > > argue that this is a bugfix, and hope that it could go into 2.6.22.
> >
> > Um, it seems rather large, and in particular adds a fair bit of
> > completely new code. Is there a simpler way of getting to an
> > acceptable point - e.g. just not use the udbg console on these boards?
>
> Yeah, that was the first approach I took. Unfortunately, currently the
> udbg console is included unconditionally on arch/powerpc. I created the
> patch below to conditionalize the use of udbg console. I thought it a
> bit risky for 2.6.22, but I think it's the right approach long term.
>
> > If we don't have udbg support for them then the udbg console would
> > seem a bit pointless, no?
>
> There is value in that with the udbg console we do see console output
> much earlier. While there's some new code (I didn't think it was all
> that much), the impact is limited to the single platform now using the
> mv64x60 console port, the prpmc2800.
You don't select PPC_UDBG for any of the cell platforms, which AFAICT
means you'll break early debugging on those.
I think you should be able to do this in terms of
CONFIG_PPC_EARLY_DEBUG, which already exists.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: publish 85xx soc dts entries as of_device on cds and ads platforms
From: Dave Jiang @ 2007-06-08 0:15 UTC (permalink / raw)
To: galak, linuxppc-dev
In-Reply-To: <20070522232849.GA26092@blade.az.mvista.com>
Dave Jiang wrote:
> Publish the devices listed in dts under SOC as of_device just like what
> mpc85xx_mds platforms do. The 85xx cds and ads platforms currently do not
> export the devices in dts as of_device.
>
> I need the memory controller, L2 cache-controller, and the PCI controller
> published as of_device so the mpc85xx EDAC driver can claim them for usage.
Since I've seen no activity in regard to this patch, can someone please tell me
if the devices in dts under the SOC entry for 85xx are suppose to be published
as of_device or is this patch wrong? I'm going to be looking at the mpc86xx
EDAC support and will have to do something similar. Am I suppose to setup
platform devices for the EDAC resources or use of_device? If of_device then I
should be publishing them similar to the mpc85xx_mds platforms right? If not,
I'll resubmit my platform device setup patch for the mpc85xx.... Thx!
> The mpc85xx EDAC driver can be found at:
> http://bluesmoke.sourceforge.net/
>
> It's in development to be pushed into the kernel.
>
> Signed-off-by: Dave Jiang <djiang@mvista.com>
>
> ---
>
> arch/powerpc/platforms/85xx/mpc85xx_ads.c | 19 +++++++++++++++++++
> arch/powerpc/platforms/85xx/mpc85xx_cds.c | 19 +++++++++++++++++++
> 2 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ads.c b/arch/powerpc/platforms/85xx/mpc85xx_ads.c
> index 5d27621..217cc13 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_ads.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_ads.c
> @@ -18,6 +18,8 @@
> #include <linux/delay.h>
> #include <linux/seq_file.h>
>
> +#include <asm/of_device.h>
> +#include <asm/of_platform.h>
> #include <asm/system.h>
> #include <asm/time.h>
> #include <asm/machdep.h>
> @@ -277,6 +279,23 @@ static int __init mpc85xx_ads_probe(void)
> return of_flat_dt_is_compatible(root, "MPC85xxADS");
> }
>
> +static struct of_device_id mpc85xx_ids[] = {
> + { .type = "soc", },
> + { .compatible = "soc", },
> + {},
> +};
> +
> +static int __init mpc85xx_publish_devices(void)
> +{
> + if (!machine_is(mpc85xx_ads))
> + return 0;
> +
> + of_platform_bus_probe(NULL, mpc85xx_ids, NULL);
> +
> + return 0;
> +}
> +device_initcall(mpc85xx_publish_devices);
> +
> define_machine(mpc85xx_ads) {
> .name = "MPC85xx ADS",
> .probe = mpc85xx_ads_probe,
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_cds.c b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
> index 1490eb3..77ef0cf 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_cds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
> @@ -26,6 +26,8 @@
> #include <linux/module.h>
> #include <linux/fsl_devices.h>
>
> +#include <asm/of_device.h>
> +#include <asm/of_platform.h>
> #include <asm/system.h>
> #include <asm/pgtable.h>
> #include <asm/page.h>
> @@ -296,6 +298,23 @@ static int __init mpc85xx_cds_probe(void)
> return of_flat_dt_is_compatible(root, "MPC85xxCDS");
> }
>
> +static struct of_device_id mpc85xx_ids[] = {
> + { .type = "soc", },
> + { .compatible = "soc", },
> + {},
> +};
> +
> +static int __init mpc85xx_publish_devices(void)
> +{
> + if (!machine_is(mpc85xx_cds))
> + return 0;
> +
> + of_platform_bus_probe(NULL, mpc85xx_ids, NULL);
> +
> + return 0;
> +}
> +device_initcall(mpc85xx_publish_devices);
> +
> define_machine(mpc85xx_cds) {
> .name = "MPC85xx CDS",
> .probe = mpc85xx_cds_probe,
>
^ permalink raw reply
* Re: [PATCH] Fix drivers/rtc/Kconfig for powerpc
From: David Gibson @ 2007-06-07 23:34 UTC (permalink / raw)
To: Jon Loeliger; +Cc: a.zummo, rtc-linux, linuxppc-dev
In-Reply-To: <1181234266.31686.37.camel@ld0161-tx32>
On Thu, Jun 07, 2007 at 11:37:46AM -0500, Jon Loeliger wrote:
> On Thu, 2007-06-07 at 11:33, Wade Farnsworth wrote:
> >
> > But since this is in drivers/rtc/Kconfig, wouldn't the maintainer of
> > that subsystem be expected to pick this up?
>
> Oh, sure. In which case he/she may be waiting on
> someone over in PowerPC land to ACK it first. :-)
> Dunno.
Sure. This patch is so simple though, I think it could go straight to
Andrew Morton.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH] 85xxCDS: Make sure restart resets the PCI bus.
From: Mark A. Greer @ 2007-06-07 22:29 UTC (permalink / raw)
To: Randy Vinson; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <46686CAF.4030407@mvista.com>
On Thu, Jun 07, 2007 at 01:38:07PM -0700, Randy Vinson wrote:
> --- a/arch/powerpc/platforms/85xx/mpc85xx_cds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
> + if ((dev = pci_get_device(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686,
> + NULL))) {
> +
> + /* Use the VIA Super Southbridge to force a PCI reset */
> + pci_read_config_byte(dev, 0x47, &tmp);
> + pci_write_config_byte(dev, 0x47, tmp | 1);
How about adding another "pci_read_config_byte(dev, 0x47, &tmp);" to
flush the write out of any fifo's and adding a comment that explains
that once that write hits the hardware, the reset is essentially
instantaneous. IOW, there isn't a race between this reset and the reset
you'd call if/when it falls out of this 'if'.
> +
> + pci_dev_put(dev);
> + }
> +
> + /*
> + * We should only get here if the P2P bridge is disabled. In that
> + * case, just use the default reset.
> + */
> + mpc85xx_restart(NULL);
Mark
^ permalink raw reply
* Re: [PATCH] Fix per-cpu allocation on oldworld SMP powermacs
From: Benjamin Herrenschmidt @ 2007-06-07 22:26 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <18023.64811.643218.556485@cargo.ozlabs.ibm.com>
On Thu, 2007-06-07 at 22:42 +1000, Paul Mackerras wrote:
> diff --git a/arch/powerpc/platforms/powermac/setup.c
> b/arch/powerpc/platforms/powermac/setup.c
> index 07b1c4e..c519b2a 100644
> --- a/arch/powerpc/platforms/powermac/setup.c
> +++ b/arch/powerpc/platforms/powermac/setup.c
> @@ -363,8 +363,19 @@ static void __init pmac_setup_arch(void)
> smp_ops = &core99_smp_ops;
> }
> #ifdef CONFIG_PPC32
> - else
> + else {
> + /*
> + * We have to set bits in cpu_possible_map here since
> the
> + * secondary CPU(s) aren't in the device tree, and
> + * setup_per_cpu_areas only allocates per-cpu data for
> + * CPUs in the cpu_possible_map.
> + */
> + int cpu;
> +
> + for (cpu = 1; cpu < 4 && cpu < NR_CPUS; ++cpu)
> + cpu_set(cpu, cpu_possible_map);
> smp_ops = &psurge_smp_ops;
> + }
> #endif
> #endif /* CONFIG_SMP */
Why not use the result from probe() instead which returns the number of
possible CPUs ? That would catch more than just the powermac case ... we
might have similar issues when finally porting PReP over...
Ben.
^ permalink raw reply
* Re: [PATCH 2/8] Add uli1575 pci-bridge sector to MPC8641HPCN dts file.
From: Benjamin Herrenschmidt @ 2007-06-07 22:12 UTC (permalink / raw)
To: Andy Fleming; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <FE5191F7-8A1D-428A-B4F0-32AD3A08B08A@freescale.com>
On Thu, 2007-06-07 at 11:21 -0500, Andy Fleming wrote:
> > The bus number shouldn't matter as long as low IO cycles are
> properly
> > forwarded to it.
>
> Right. Unless there's an interrupt mapping, which has to include
> the
> bus number in the filter. When that happens, parsing the interrupt
> map for the i8259's mapping fails because its address doesn't match.
Hrm... does the interrupt-map include the bus number for you ? They
generally don't ...
Ben.
^ permalink raw reply
* [PATCH 2/2] Donate dedicated CPU cycles
From: Jake Moilanen @ 2007-06-07 21:30 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <18022.14166.83442.173369@cargo.ozlabs.ibm.com>
A Power6 can give up CPU cycles on a dedicated CPU (as opposed to a
shared CPU) to other shared processors if the administrator asks for it
(via the HMC).
This patch enables that to work properly on P6.
This just involves setting a bit in the CAS structure as well as the
VPA. To donate cycles, a CPU has to have all SMT threads idle and w/
donate bit set in the VPA. Then call H_CEDE.
The reason why shared processors just aren't used is because dedicated
CPUs are guaranteed an actual processor, yet the system is still able to
increase the capacity of the shared CPU pool.
Signed-off-by: Jake Moilanen <moilanen@austin.ibm.com>
--
arch/powerpc/kernel/prom_init.c | 4 +++-
arch/powerpc/platforms/pseries/setup.c | 2 ++
2 files changed, 5 insertions(+), 1 deletion(-)
Index: powerpc/arch/powerpc/kernel/prom_init.c
===================================================================
--- powerpc.orig/arch/powerpc/kernel/prom_init.c
+++ powerpc/arch/powerpc/kernel/prom_init.c
@@ -635,6 +635,7 @@ static void __init early_cmdline_parse(v
/* ibm,dynamic-reconfiguration-memory property supported */
#define OV5_DRCONF_MEMORY 0x20
#define OV5_LARGE_PAGES 0x10 /* large pages supported */
+#define OV5_DONATE_DEDICATE_CPU 0x02 /* donate dedicated CPU support */
/* PCIe/MSI support. Without MSI full PCIe is not supported */
#ifdef CONFIG_PCI_MSI
#define OV5_MSI 0x01 /* PCIe/MSI support */
@@ -685,7 +686,8 @@ static unsigned char ibm_architecture_ve
/* option vector 5: PAPR/OF options */
3 - 2, /* length */
0, /* don't ignore, don't halt */
- OV5_LPAR | OV5_SPLPAR | OV5_LARGE_PAGES | OV5_DRCONF_MEMORY | OV5_MSI,
+ OV5_LPAR | OV5_SPLPAR | OV5_LARGE_PAGES | OV5_DRCONF_MEMORY |
+ OV5_DONATE_DEDICATE_CPU | OV5_MSI,
};
/* Old method - ELF header with PT_NOTE sections */
Index: powerpc/arch/powerpc/platforms/pseries/setup.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pseries/setup.c
+++ powerpc/arch/powerpc/platforms/pseries/setup.c
@@ -399,6 +399,7 @@ static void pseries_dedicated_idle_sleep
* a good time to find other work to dispatch.
*/
get_lppaca()->idle = 1;
+ get_lppaca()->donate_dedicated_cpu = 1;
/*
* We come in with interrupts disabled, and need_resched()
@@ -431,6 +432,7 @@ static void pseries_dedicated_idle_sleep
out:
HMT_medium();
+ get_lppaca()->donate_dedicated_cpu = 0;
get_lppaca()->idle = 0;
}
^ permalink raw reply
* [PATCH 1/2] Donate dedicated CPU cycles
From: Jake Moilanen @ 2007-06-07 21:27 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <18022.14166.83442.173369@cargo.ozlabs.ibm.com>
Rename the VPA's cpuctls_task_attrs field to a more accurate name.
Signed-off-by: Jake Moilanen <moilanen@austin.ibm.com>
--
include/asm-powerpc/lppaca.h | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
Index: powerpc/include/asm-powerpc/lppaca.h
===================================================================
--- powerpc.orig/include/asm-powerpc/lppaca.h
+++ powerpc/include/asm-powerpc/lppaca.h
@@ -98,7 +98,7 @@ struct lppaca {
u64 saved_gpr5; // Saved GPR5 x30-x37
u8 reserved4; // Reserved x38-x38
- u8 cpuctls_task_attrs; // Task attributes for cpuctls x39-x39
+ u8 donate_dedicated_cpu; // Donate dedicated CPU cycles x39-x39
u8 fpregs_in_use; // FP regs in use x3A-x3A
u8 pmcregs_in_use; // PMC regs in use x3B-x3B
volatile u32 saved_decr; // Saved Decr Value x3C-x3F
^ permalink raw reply
* Re: [PATCH] Donate dedicated CPU cycles
From: Jake Moilanen @ 2007-06-07 21:24 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <18022.14166.83442.173369@cargo.ozlabs.ibm.com>
On Wed, 2007-06-06 at 14:25 +1000, Paul Mackerras wrote:
> Jake Moilanen writes:
>
> > A Power6 can give up CPU cycles on a dedicated CPU (as opposed to a
> > shared CPU) to other shared processors if the administrator asks for it
> > (via the HMC).
> >
> > This patch enables that to work properly on P6.
>
> Is this something that needs to be controlled from userland, e.g. by a
> sysctl or something in /sys?
The turning on of donating of dedicated CPU cycles is done via the HMC.
IHMO, it would be a bit redundant to do it in two places.
The reason why this is better than just shared processors is the
partitions w/ dedicated CPUs are guaranteed an actual processor, yet the
system is still able to increase the capacity of the shared CPU pool.
I have an updated patch come w/ a rename of the VPA field.
Jake
^ permalink raw reply
* [PATCH] 85xxCDS: Make sure restart resets the PCI bus.
From: Randy Vinson @ 2007-06-07 20:38 UTC (permalink / raw)
To: linuxppc-dev@ozlabs.org
The current 85xxCDS restart code fails to reset the PCI bus which can
lead to odd behavior after the restart. This patch uses the VIA Super
Southbridge to perform a PCI reset which will reset the entire system.
NOTE: Since the VIA chip is behind a PCI-to-PCI bridge which can be
disabled with a switch setting, it may not be possible to perform the
PCI bus reset. In this case, the code defaults to the previous restart
mechanism.
Signed-off-by: Randy Vinson <rvinson@mvista.com>
---
arch/powerpc/platforms/85xx/mpc85xx_cds.c | 26 ++++++++++++++++++++++++++
1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_cds.c b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
index 431aaa2..f1eaf07 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_cds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
@@ -62,6 +62,28 @@ static volatile u8 *cadmus;
extern int mpc85xx_pci2_busno;
+static void mpc85xx_cds_restart(char *cmd)
+{
+ struct pci_dev *dev;
+ u_char tmp;
+
+ if ((dev = pci_get_device(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686,
+ NULL))) {
+
+ /* Use the VIA Super Southbridge to force a PCI reset */
+ pci_read_config_byte(dev, 0x47, &tmp);
+ pci_write_config_byte(dev, 0x47, tmp | 1);
+
+ pci_dev_put(dev);
+ }
+
+ /*
+ * We should only get here if the P2P bridge is disabled. In that
+ * case, just use the default reset.
+ */
+ mpc85xx_restart(NULL);
+}
+
static int mpc85xx_exclude_device(u_char bus, u_char devfn)
{
if (bus == 0 && PCI_SLOT(devfn) == 0)
@@ -327,7 +349,11 @@ define_machine(mpc85xx_cds) {
.init_IRQ = mpc85xx_cds_pic_init,
.show_cpuinfo = mpc85xx_cds_show_cpuinfo,
.get_irq = mpic_get_irq,
+#ifdef CONFIG_PCI
+ .restart = mpc85xx_cds_restart,
+#else
.restart = mpc85xx_restart,
+#endif
.calibrate_decr = generic_calibrate_decr,
.progress = udbg_progress,
};
--
1.5.0.GIT
^ permalink raw reply related
* Re: how are the interrupts mapped on the taiga board?
From: Sergei Shtylyov @ 2007-06-07 20:17 UTC (permalink / raw)
To: Leisner, Martin; +Cc: Lund, Nathan, linuxppc-embedded
In-Reply-To: <556445368AFA1C438794ABDA8901891C066B5AC5@usa0300ms03.na.xerox.net>
Leisner, Martin wrote:
> I'm looking at mpc7448hpc2.dts -- and the tsi108 specs...
> I see how TTYS0 is IRQ12 and the ethernet is IRQ16...that agrees with
> the file...
> and the tundra TSi108 spec.
> But in after we boot in /proc/interrupts:
>
> 16: 1796 Tsi108_PIC Edge serial
> 18: 24148 Tsi108_PIC Level eth0
> I don't see in the .dts a mapping from 12->16 and 16->18.
> Nore have I found anything in the source...
> I using 2.6.21.1.
> How is this done?
It's done by arch/powerpc kernel itself which remaps any IRQs < 16.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] Fix the LPC47M192 SuperIO on the MPC8641 HPCN
From: Andy Fleming @ 2007-06-07 20:01 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, Arnd Bergmann, paulus
In-Reply-To: <63ff9dee6b212cc7908c8e1b73920ea6@kernel.crashing.org>
On Jun 7, 2007, at 11:35, Segher Boessenkool wrote:
>>> Hardcoded I/O port numbers always worry me a little. I know that
>>> this
>>> is
>>> supposed to work in general, but can't you read the I/O port range
>>> from
>>> a device tree property?
>>
>> I suppose I could create a device node for the Super I/O config
>> registers and use those instead of hardcoding it here.
>
> I'd just hide it all, do this setup in the firmware,
> where it belongs, and don't expose the superio config
> in the device tree.
No more. No more firmware-only initializations. It sounds great, in
principle, until you actually have to figure out why someone's setup
isn't working. I'm tired of having to see if the dts, u-boot, and
Linux are in sync. If Linux wants to use a device, I think it's not
unreasonable to have it setup the device itself. That way, Linux can
do whatever it wants with the device, and not have to rely on U-Boot
(or some other firmware) setting up the appropriate bits.
>
>> superio_cfg@4e {
>> reg = <1 4e 2>;
>> compatible = "smsc-lpc47m192-cfg";
>> };
>>
>> I'm not sure if the name and compatible properties are appropriate
>> though. Any recommendations?
>
> "superio" and "smsc,lpc47m192" I'd say. You also
> then should link the logical devices on the superio
> to the device nodes that represent those. I'm not
> sure this is all worth it, this is low-level setup
> the firmware should do and everything else can treat
> it as a black box.
Please...no. What happens next is that we find a small bug that
requires we modify U-Boot to do the initialization slightly
differently, and then requires Linux to act slightly differently.
And then I get emails every day from people wondering why their
boards don't work.
Andy
^ permalink raw reply
* Re: [RFC] 85XX: Allow 8259 cascade to share an MPIC interrupt line.
From: Sergei Shtylyov @ 2007-06-07 20:04 UTC (permalink / raw)
To: Randy Vinson; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <4668628A.30401@mvista.com>
Randy Vinson wrote:
>>>After processing a possible 8259 interrupt, the 8259 cascade handler
>>>calls handle_fasteoi_irq to perform the actions noted above. However,
>>>with the 8259 cascade handler hooked to the interrupt, the threaded IRQ
>>>handler doesn't recognize it as one of the 4 standard generic IRQ types
>>>and calls the threaded version of do_irq instead of the threaded fasteoi
>>>handler.
>> Ah, that's it!
>>>Once the threaded do_irq handler completes the action list, it
>>>uses the .end routine to restart the interrupt flow. Pointing .end to
>>>the standard MPIC unmask routine allows it to balance the interrupt mask
>>>operation performed by handle_fasteoi_irq before it scheduled the IRQ
>>>thread.
>> Well, but when MPIC's eoi() method is called then? :-O
> It's called from handle_fasteoi_irq as I described in the "basics" ;)
Indeed, I somehow thought eoi() is out of the threaded exit path of
handle_fasteoi_irq() -- but it hasn't always been that way! So, it's probably
a painful memories of my past quarrel with Ingo over fasteoi breakage that
played a trick with me here. ;-)
> Randy V.
WBR, Sergei
^ permalink raw reply
* how are the interrupts mapped on the taiga board?
From: Leisner, Martin @ 2007-06-07 19:55 UTC (permalink / raw)
To: linuxppc-embedded; +Cc: Lund, Nathan
I'm looking at mpc7448hpc2.dts -- and the tsi108 specs...
I see how TTYS0 is IRQ12 and the ethernet is IRQ16...that agrees with
the file...
and the tundra TSi108 spec.
But in after we boot in /proc/interrupts:
16: 1796 Tsi108_PIC Edge serial
18: 24148 Tsi108_PIC Level eth0
I don't see in the .dts a mapping from 12->16 and 16->18.
Nore have I found anything in the source...
I using 2.6.21.1.
How is this done?
marty
^ permalink raw reply
* Re: [RFC] 85XX: Allow 8259 cascade to share an MPIC interrupt line.
From: Randy Vinson @ 2007-06-07 19:54 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <46685F77.7050901@ru.mvista.com>
Sergei Shtylyov wrote:
[snippage]
>> The threaded interrupt system has threaded routines for the 4 standard
>> interrupts types used by the generic IRQ system (fasteoi, edge, level
>> and simple), plus a handler for non-generic interrupts (the ones that
>> still use __do_IRQ.) When interrupts are being threaded and a
>> non-cascaded MPIC interrupt occurs, desc->handler normally points to
>> handle_fasteoi_irq which masks the interrupt source, schedules the
>> corresponding IRQ thread, issues an EOI and returns to the interrupted
>> context. At a later time, the scheduler dispatches the IRQ thread which
>> calls a threaded version of the fasteoi handler. The threaded fasteoi
>> routine processes the action chain and calls .unmask to re-enable the
>> interrupt before returning to the calling thread. Once the interrupt has
>> been unmasked, the entire process is free to repeat as needed.
>
> Oh, you could have really skipped the basics. ;-)
Maybe not. See below.
>
>> After processing a possible 8259 interrupt, the 8259 cascade handler
>> calls handle_fasteoi_irq to perform the actions noted above. However,
>> with the 8259 cascade handler hooked to the interrupt, the threaded IRQ
>> handler doesn't recognize it as one of the 4 standard generic IRQ types
>> and calls the threaded version of do_irq instead of the threaded fasteoi
>> handler.
>
> Ah, that's it!
>
>> Once the threaded do_irq handler completes the action list, it
>> uses the .end routine to restart the interrupt flow. Pointing .end to
>> the standard MPIC unmask routine allows it to balance the interrupt mask
>> operation performed by handle_fasteoi_irq before it scheduled the IRQ
>> thread.
>
> Well, but when MPIC's eoi() method is called then? :-O
It's called from handle_fasteoi_irq as I described in the "basics" ;)
Randy V.
^ permalink raw reply
* Re: [RFC] 85XX: Allow 8259 cascade to share an MPIC interrupt line.
From: Sergei Shtylyov @ 2007-06-07 19:41 UTC (permalink / raw)
To: Randy Vinson; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <46685B9C.10501@mvista.com>
Randy Vinson wrote:
>>>---
>>>Note that there may very well be a better way of accomplishing this.
>>>If someone
>>>has a better alternative, I'm open to it. This was just the simplest
>>>way I could
>>>get this to work.
>>>Also, the addition of the .end routine for the MPIC is not strictly
>>>necessary for
>>>this patch. It's there so this code will run from within the threaded
>>>interrupt
>>>context used by the Real Time
>> Hmmm, why would you need that? Where RT is different from the
>>vanilla? :-O
> The threaded interrupt system has threaded routines for the 4 standard
> interrupts types used by the generic IRQ system (fasteoi, edge, level
> and simple), plus a handler for non-generic interrupts (the ones that
> still use __do_IRQ.) When interrupts are being threaded and a
> non-cascaded MPIC interrupt occurs, desc->handler normally points to
> handle_fasteoi_irq which masks the interrupt source, schedules the
> corresponding IRQ thread, issues an EOI and returns to the interrupted
> context. At a later time, the scheduler dispatches the IRQ thread which
> calls a threaded version of the fasteoi handler. The threaded fasteoi
> routine processes the action chain and calls .unmask to re-enable the
> interrupt before returning to the calling thread. Once the interrupt has
> been unmasked, the entire process is free to repeat as needed.
Oh, you could have really skipped the basics. ;-)
> After processing a possible 8259 interrupt, the 8259 cascade handler
> calls handle_fasteoi_irq to perform the actions noted above. However,
> with the 8259 cascade handler hooked to the interrupt, the threaded IRQ
> handler doesn't recognize it as one of the 4 standard generic IRQ types
> and calls the threaded version of do_irq instead of the threaded fasteoi
> handler.
Ah, that's it!
> Once the threaded do_irq handler completes the action list, it
> uses the .end routine to restart the interrupt flow. Pointing .end to
> the standard MPIC unmask routine allows it to balance the interrupt mask
> operation performed by handle_fasteoi_irq before it scheduled the IRQ
> thread.
Well, but when MPIC's eoi() method is called then? :-O
>>> arch/powerpc/platforms/85xx/mpc85xx_cds.c | 32
>>>+++++++++++++++++++++++++---
>>> arch/powerpc/sysdev/mpic.c | 1 +
>>> 2 files changed, 29 insertions(+), 4 deletions(-)
>>>diff --git a/arch/powerpc/platforms/85xx/mpc85xx_cds.c
>>>b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
>>>index 1490eb3..431aaa2 100644
>>>--- a/arch/powerpc/platforms/85xx/mpc85xx_cds.c
>>>+++ b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
>>>@@ -127,16 +127,30 @@ static void __init mpc85xx_cds_pcibios_fixup(void)
>>> }
>>> #ifdef CONFIG_PPC_I8259
>>>-#warning The i8259 PIC support is currently broken
>>>-static void mpc85xx_8259_cascade(unsigned int irq, struct irq_desc
>>>*desc)
>>>+static void mpc85xx_8259_cascade_handler(unsigned int irq,
>>>+ struct irq_desc *desc)
>>> {
>>> unsigned int cascade_irq = i8259_irq();
>>>
>>> if (cascade_irq != NO_IRQ)
>>>+ /* handle an interrupt from the 8259 */
>>> generic_handle_irq(cascade_irq);
>>>
>>>- desc->chip->eoi(irq);
>>>+ /* check for any interrupts from the shared IRQ line */
>>>+ handle_fasteoi_irq(irq, desc);
>>> }
>>>+
>>>+static irqreturn_t mpc85xx_8259_cascade_action(int irq, void *dev_id)
>>>+{
>>>+ return IRQ_HANDLED;
>>>+}
>> Well, mpc85xx_8259_intr() would probably be more in line with the
>>code elsewhere... and you could keep the mpc85xx_8259_cascade() name then.
> I chose the names to show that the two routines run at different levels
> within the interrupt subsystem. The cascade handler always runs in
> interrupt context while the cascade action can run in interrupt context
> or in threaded context. While I was researching this, I kept getting
> confused between the desc->handler and action->handler fields so I named
> them different to make it easier for me to keep the differences clear in
> my head. I can change the names it if you think it's important.
I don't insist. :-)
> Randy V.
WBR, Sergei
^ permalink raw reply
* Re: [spi-devel-general] [PATCH] Simple driver for Xilinx SPI controler.
From: David Brownell @ 2007-06-07 19:21 UTC (permalink / raw)
To: Andrei Konovalov; +Cc: spi-devel-general, linuxppc-embedded
In-Reply-To: <466850FB.5030505@ru.mvista.com>
On Thursday 07 June 2007, Andrei Konovalov wrote:
> >> +/* Simple macros to get the code more readable */
> >> +#define xspi_in16(addr) in_be16((u16 __iomem *)(addr))
> >> +#define xspi_in32(addr) in_be32((u32 __iomem *)(addr))
> >> +#define xspi_out16(addr, value) out_be16((u16 __iomem *)(addr), (value))
> >> +#define xspi_out32(addr, value) out_be32((u32 __iomem *)(addr), (value))
> >
> > I'm rather used to seeing I/O addressses passed around as "void __iomem *"
> > so those sorts of cast are not needed... :)
>
> I've decided to make the base_address (u8 __iomem *) to make it easier to
> add the register offsets to the base. Like that:
Adding register offsets works with "void __iomem *" too ...
> > You should not need an abort primitive. ...
>
> I am paranoid enough not to rely 100% on the spi_device's and the spi_bitbang
> doing all that :)
> Agreed, xspi_abort_transfer is bad name here. But I still would like to
> leave these two writes:
>
> + /* Deselect the slave on the SPI bus */
> + xspi_out32(regs_base + XSPI_SSR_OFFSET, 0xffff);
> + /* Disable the transmitter */
> + xspi_out16(regs_base + XSPI_CR_OFFSET,
> + XSPI_CR_TRANS_INHIBIT | XSPI_CR_MANUAL_SSELECT);
>
> in xilinx_spi_remove(). Just in case :)
What happens when you add a BUG_ON to catch the device still being active?
Really, you need to rely on the rest of the system working correctly.
> > I take it you can't support SPI_CS_HIGH??
>
> There is no clear indication in the SPI controller docs that SPI_CS_HIGH
> would work. I suspect it would as we don't use the ability of the controller
> to generate the chip selects automatically (the auto mode doesn't support SPI_CS_HIGH).
> But it is more safe to advertise SPI_CS_HIGH is not supported.
If you don't explicitly support it, it's unlikely to work. :)
- Dave
^ permalink raw reply
* Re: [RFC] 85XX: Allow 8259 cascade to share an MPIC interrupt line.
From: Randy Vinson @ 2007-06-07 19:25 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <46683D12.1050204@ru.mvista.com>
Sergei Shtylyov wrote:
> Hello.
Hi.
>
> Randy Vinson wrote:
>
>> The Freescale MPC8555CDS and MPC8548CDS reference hardware has a legacy
>> 8259 interrupt controller pair contained within a VIA VT82C686B
>> Southbridge
>> on the main carrier board. The processor complex plugs into the carrier
>> card using a PCI slot which limits the available interrupts to the
>> INTA-INTD PCI interrupts. The output of the 8259 cascade pair is routed
>> through a gate array and connected to the PCI INTA interrupt line.
>> The normal interrupt chaining hook (set_irq_chained_handler) does
>> not allow sharing of the chained interrupt which prevents the
>> use of PCI INTA by PCI devices. This patch allows the 8259 cascade
>> pair to share their interrupt line with PCI devices.
>
> Hmm, I see you've come up with an interesting solution. :-)
Well, I'm not sure how interesting it is, but it works :)
>
>> Signed-off-by: Randy Vinson <rvinson@mvista.com>
>> ---
>> Note that there may very well be a better way of accomplishing this.
>> If someone
>> has a better alternative, I'm open to it. This was just the simplest
>> way I could
>> get this to work.
>>
>> Also, the addition of the .end routine for the MPIC is not strictly
>> necessary for
>> this patch. It's there so this code will run from within the threaded
>> interrupt
>> context used by the Real Time
>
> Hmmm, why would you need that? Where RT is different from the
> vanilla? :-O
The threaded interrupt system has threaded routines for the 4 standard
interrupts types used by the generic IRQ system (fasteoi, edge, level
and simple), plus a handler for non-generic interrupts (the ones that
still use __do_IRQ.) When interrupts are being threaded and a
non-cascaded MPIC interrupt occurs, desc->handler normally points to
handle_fasteoi_irq which masks the interrupt source, schedules the
corresponding IRQ thread, issues an EOI and returns to the interrupted
context. At a later time, the scheduler dispatches the IRQ thread which
calls a threaded version of the fasteoi handler. The threaded fasteoi
routine processes the action chain and calls .unmask to re-enable the
interrupt before returning to the calling thread. Once the interrupt has
been unmasked, the entire process is free to repeat as needed.
After processing a possible 8259 interrupt, the 8259 cascade handler
calls handle_fasteoi_irq to perform the actions noted above. However,
with the 8259 cascade handler hooked to the interrupt, the threaded IRQ
handler doesn't recognize it as one of the 4 standard generic IRQ types
and calls the threaded version of do_irq instead of the threaded fasteoi
handler. Once the threaded do_irq handler completes the action list, it
uses the .end routine to restart the interrupt flow. Pointing .end to
the standard MPIC unmask routine allows it to balance the interrupt mask
operation performed by handle_fasteoi_irq before it scheduled the IRQ
thread.
>
>> arch/powerpc/platforms/85xx/mpc85xx_cds.c | 32
>> +++++++++++++++++++++++++---
>> arch/powerpc/sysdev/mpic.c | 1 +
>> 2 files changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_cds.c
>> b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
>> index 1490eb3..431aaa2 100644
>> --- a/arch/powerpc/platforms/85xx/mpc85xx_cds.c
>> +++ b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
>> @@ -127,16 +127,30 @@ static void __init mpc85xx_cds_pcibios_fixup(void)
>> }
>>
>> #ifdef CONFIG_PPC_I8259
>> -#warning The i8259 PIC support is currently broken
>> -static void mpc85xx_8259_cascade(unsigned int irq, struct irq_desc
>> *desc)
>> +static void mpc85xx_8259_cascade_handler(unsigned int irq,
>> + struct irq_desc *desc)
>> {
>> unsigned int cascade_irq = i8259_irq();
>>
>> if (cascade_irq != NO_IRQ)
>> + /* handle an interrupt from the 8259 */
>> generic_handle_irq(cascade_irq);
>>
>> - desc->chip->eoi(irq);
>> + /* check for any interrupts from the shared IRQ line */
>> + handle_fasteoi_irq(irq, desc);
>> }
>> +
>> +static irqreturn_t mpc85xx_8259_cascade_action(int irq, void *dev_id)
>> +{
>> + return IRQ_HANDLED;
>> +}
>
> Well, mpc85xx_8259_intr() would probably be more in line with the
> code elsewhere... and you could keep the mpc85xx_8259_cascade() name then.
I chose the names to show that the two routines run at different levels
within the interrupt subsystem. The cascade handler always runs in
interrupt context while the cascade action can run in interrupt context
or in threaded context. While I was researching this, I kept getting
confused between the desc->handler and action->handler fields so I named
them different to make it easier for me to keep the differences clear in
my head. I can change the names it if you think it's important.
Randy V.
^ permalink raw reply
* Re: [patch 07/18] PS3: Make ps3av.h usable from user space
From: Christoph Hellwig @ 2007-06-07 19:15 UTC (permalink / raw)
To: Geoff Levand
Cc: Geert Uytterhoeven, linuxppc-dev, Paul Mackerras,
Christoph Hellwig, Masashi Kimoto
In-Reply-To: <4666E453.2000302@am.sony.com>
On Wed, Jun 06, 2007 at 09:44:03AM -0700, Geoff Levand wrote:
> Christoph Hellwig wrote:
> > On Tue, Jun 05, 2007 at 08:00:07PM -0700, Geoff Levand wrote:
> >> The user applications to manage the PS3 AV modes can use values
> >> defined in this header.
> >
> > NACK. First please don't introduce new unifdef-y headers but always
> > separated them. Second I don't see any of the values actually used
> > in a user<->kerne interface. If the application only happens to use
> > the same values it should ship a copy of the header intead.
>
> Yes, they are currently used with ioctl for video mode selection
> by the ps3videomode utility.
>
> OK, you recommend having two headers one for the kernel and one for
> the application or library, then keeping them in sync. I think
> in some ways that is easier for the package user also, since there
> is no dependency on kernel headers. It does mean that the package
> definitions need to be kept up to date.
Now that we're actually using it for the ioctls it's fine to export
it. But please use a separate header that contains just these bits
instead of introducing more headers that need unifdefing.
^ permalink raw reply
* Re: [spi-devel-general] [PATCH] Simple driver for Xilinx SPI controler.
From: Andrei Konovalov @ 2007-06-07 18:39 UTC (permalink / raw)
To: David Brownell; +Cc: spi-devel-general, linuxppc-embedded
In-Reply-To: <200706062209.09731.david-b@pacbell.net>
Hi David,
Thanks for reviewing this stuff!
David Brownell wrote:
> On Wednesday 06 June 2007, Andrei Konovalov wrote:
>> Would be nice to get this driver into mainline.
>> Reviews and comments are welcome.
>
> I'll ignore the Kconfig flamage ... ;)
>
>
>> --- /dev/null
>> +++ b/drivers/spi/xilinx_spi.c
>> @@ -0,0 +1,447 @@
>> +/*
>> + * xilinx_spi.c
>> + *
>> + * Xilinx SPI controler driver (master mode only)
>> + *
>> + * Author: MontaVista Software, Inc.
>> + * source@mvista.com
>> + *
>> + * 2002-2007 (c) MontaVista Software, Inc. This file is licensed under the
>> + * terms of the GNU General Public License version 2. This program is licensed
>> + * "as is" without any warranty of any kind, whether express or implied.
>> + */
>> +
>> +
>> +/* Simple macros to get the code more readable */
>> +#define xspi_in16(addr) in_be16((u16 __iomem *)(addr))
>> +#define xspi_in32(addr) in_be32((u32 __iomem *)(addr))
>> +#define xspi_out16(addr, value) out_be16((u16 __iomem *)(addr), (value))
>> +#define xspi_out32(addr, value) out_be32((u32 __iomem *)(addr), (value))
>
> I'm rather used to seeing I/O addressses passed around as "void __iomem *"
> so those sorts of cast are not needed... :)
I've decided to make the base_address (u8 __iomem *) to make it easier to
add the register offsets to the base. Like that:
static void xspi_init_hw(u8 __iomem *regs_base)
{
/* Reset the SPI device */
xspi_out32(regs_base + XIPIF_V123B_RESETR_OFFSET,
XIPIF_V123B_RESET_MASK);
...
Without the cast, out_be32(regs_base + XIPIF_V123B_RESETR_OFFSET, XIPIF_V123B_RESET_MASK)
produces a warning.
Another solution could be
#define XSPI_REG(base, offset) (void *)((base) + (offset))
and
/* Reset the SPI device */
out_be32(XSPI_REG(regs_base, XIPIF_V123B_RESETR_OFFSET),
XIPIF_V123B_RESET_MASK);
Is it better?
>> +
>> +static void xspi_abort_transfer(u8 __iomem *regs_base)
>> +{
>
> You should not need an abort primitive. This is called only
> in the remove-controller path. By the time it's called,
> every child spi_device on this bus segment should have been
> removed ... which means any spi_driver attached to that
> device has already returned from its remove() method, which
> in turn means that there will be no spi_message objects in
> flight from any of those drivers.
I am paranoid enough not to rely 100% on the spi_device's and the spi_bitbang
doing all that :)
Agreed, xspi_abort_transfer is bad name here. But I still would like to
leave these two writes:
+ /* Deselect the slave on the SPI bus */
+ xspi_out32(regs_base + XSPI_SSR_OFFSET, 0xffff);
+ /* Disable the transmitter */
+ xspi_out16(regs_base + XSPI_CR_OFFSET,
+ XSPI_CR_TRANS_INHIBIT | XSPI_CR_MANUAL_SSELECT);
in xilinx_spi_remove(). Just in case :)
>> +static void xilinx_spi_chipselect(struct spi_device *spi, int is_on)
>> +{
>> + struct xilinx_spi *xspi;
>> + u8 __iomem *regs_base;
>> +
>> + xspi = spi_master_get_devdata(spi->master);
>> + regs_base = xspi->regs;
>> +
>> + if (is_on == BITBANG_CS_INACTIVE) {
>> + /* Deselect the slave on the SPI bus */
>> + xspi_out32(regs_base + XSPI_SSR_OFFSET, 0xffff);
>
> I take it you can't support SPI_CS_HIGH??
There is no clear indication in the SPI controller docs that SPI_CS_HIGH
would work. I suspect it would as we don't use the ability of the controller
to generate the chip selects automatically (the auto mode doesn't support SPI_CS_HIGH).
But it is more safe to advertise SPI_CS_HIGH is not supported.
>> +/* spi_bitbang requires custom setup_transfer() to be defined if there is a
>> + * custom txrx_bufs(). We have nothing to setup here as the SPI IP block
>> + * supports just 8 bits per word, and SPI clock can't be changed in software.
>> + * Check for 8 bits per word; speed_hz checking could be added if the SPI
>> + * clock information is available. Chip select delay calculations could be
>> + * added here as soon as bitbang_work() can be made aware of the delay value.
>> + */
>> +static int xilinx_spi_setup_transfer(struct spi_device *spi,
>> + struct spi_transfer *t)
>> +{
>> + u8 bits_per_word;
>> +
>> + bits_per_word = (t) ? t->bits_per_word : spi->bits_per_word;
>> + if (bits_per_word != 8)
>> + return -EINVAL;
>
> Speed checking *SHOULD* be added; the clock info can be platform data.
>
> It would make trouble if a driver needed to turn the clock down to,
> say, 400 KHz for some operation, and the controller said "yes" but
> kept it at 25 MHz or whatever. It's OK if the driver is told that
> 400 KHz can't happen though -- it's possible to recover from that.
OK
> (Although in practice it's best to have the transfer method do
> the error checking, so that messages that will fail do so before
> they are allowed to enter the I/O queue.)
>
> ISTR you may need to delegate to the default method here too, but
> it's been a while since I poked at that level and the issue might
> not apply to this particular driver config.
>
>
>
>> +
>> + return 0;
>> +}
>> +
>> +
>> +static int xilinx_spi_setup(struct spi_device *spi)
>> +{
>> + struct spi_bitbang *bitbang;
>> + struct xilinx_spi *xspi;
>> + int retval;
>> +
>> + xspi = spi_master_get_devdata(spi->master);
>> + bitbang = &xspi->bitbang;
>
> You need to verify ALL the input parameters. In particular,
> mask spi->mode against all the values this driver recognizes
> and supports. If you don't support SPI_LSB_FIRST it's a bug
> if setup() succeeds after setting that. Same thing with all
> other bits defined today (SPI_3WIRE, SPI_CS_HIGH) and in the
> future...
>
> For an example, see the atmel_spi.c driver and MODEBITS.
> There's a patch in MM that makes all the SPI controller
> drivers have analagous tests ... and for bitbang there's
> a patch adding spi_bitbang.mode flags to declare any
> extra spi->mode flags that should be accepted.
OK. Thanks.
>> + ...
>> +
>> +static irqreturn_t xilinx_spi_irq(int irq, void *dev_id)
>> +{
>> + struct xilinx_spi *xspi;
>> + u8 __iomem *regs_base;
>> + u32 ipif_isr;
>> +
>> + xspi = (struct xilinx_spi *) dev_id;
>> + regs_base = xspi->regs;
>> +
>> + /* Get the IPIF inetrrupts, and clear them immediately */
>
> Spell checkers will tell you this is "interrupts" ... ;)
Oops...
>> + ipif_isr = xspi_in32(regs_base + XIPIF_V123B_IISR_OFFSET);
>> + xspi_out32(regs_base + XIPIF_V123B_IISR_OFFSET, ipif_isr);
>> +
>> + if (ipif_isr & XSPI_INTR_TX_EMPTY) { /* Transmission completed */
>> + u16 cr;
>> + u8 sr;
>> +
>> + /* A transmit has just completed. Process received data and
>> + * check for more data to transmit. Always inhibit the
>> + * transmitter while the Isr refills the transmit register/FIFO,
>> + * or make sure it is stopped if we're done.
>> + */
>> + cr = xspi_in16(regs_base + XSPI_CR_OFFSET);
>> + xspi_out16(regs_base + XSPI_CR_OFFSET,
>> + cr | XSPI_CR_TRANS_INHIBIT);
>> +
>> + /* Read out all the data from the Rx FIFO */
>> + sr = in_8(regs_base + XSPI_SR_OFFSET);
>> + while ((sr & XSPI_SR_RX_EMPTY_MASK) == 0) {
>> + u8 data;
>> +
>> + data = in_8(regs_base + XSPI_RXD_OFFSET);
>> + if (xspi->rx_ptr) {
>> + *xspi->rx_ptr++ = data;
>> + }
>> + sr = in_8(regs_base + XSPI_SR_OFFSET);
>> + }
>> +
>> + /* See if there is more data to send */
>> + if (xspi->remaining_bytes > 0) {
>> + /* sr content is valid here; no need for io_8() */
>> + while ((sr & XSPI_SR_TX_FULL_MASK) == 0
>> + && xspi->remaining_bytes > 0) {
>> + if (xspi->tx_ptr) {
>> + out_8(regs_base + XSPI_TXD_OFFSET,
>> + *xspi->tx_ptr++);
>> + } else {
>> + out_8(regs_base + XSPI_TXD_OFFSET, 0);
>> + }
>
> This duplicates the loop in txrx_bufs(); that's bad style.
> Have one routine holding the shared code.
OK
>> +static int __init xilinx_spi_probe(struct platform_device *dev)
>> +{
>> + int ret = 0;
>> + struct spi_master *master;
>> + struct xilinx_spi *xspi;
>> + struct xspi_platform_data *pdata;
>> + struct resource *r;
>> +
>> + /* Get resources(memory, IRQ) associated with the device */
>> + master = spi_alloc_master(&dev->dev, sizeof(struct xilinx_spi));
>> +
>> + if (master == NULL) {
>> + return -ENOMEM;
>> + }
>> +
>> + platform_set_drvdata(dev, master);
>> + pdata = dev->dev.platform_data;
>> +
>> + if (pdata == NULL) {
>> + ret = -ENODEV;
>> + goto put_master;
>> + }
>> +
>> + r = platform_get_resource(dev, IORESOURCE_MEM, 0);
>> + if (r == NULL) {
>> + ret = -ENODEV;
>> + goto put_master;
>> + }
>> +
>> + xspi = spi_master_get_devdata(master);
>> + xspi->bitbang.master = spi_master_get(master);
>> + xspi->bitbang.chipselect = xilinx_spi_chipselect;
>> + xspi->bitbang.setup_transfer = xilinx_spi_setup_transfer;
>> + xspi->bitbang.txrx_bufs = xilinx_spi_txrx_bufs;
>> + xspi->bitbang.master->setup = xilinx_spi_setup;
>> + init_completion(&xspi->done);
>> +
>> + xspi->regs = ioremap(r->start, r->end - r->start + 1);
>
> Strictly speaking a request_region() should precede the ioremap,
> but a lot of folk don't bother. However, lacking that I'd put
> the request_irq() earlier, since that will be the only resource
> providing any guard against another driver sharing the hardware.
Will add request_region(). I was confused by platform_device_register()
doing something with the resources (contrary to the "plain" device_register()).
Thanks,
Andrei
^ 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