* Re: [PATCH] powerpc: EOI spurious irqs during boot so they can be reenabled later
From: Michael Ellerman @ 2008-08-06 4:53 UTC (permalink / raw)
To: miltonm; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <489912b6.cc.1809.1931903668@bga.com>
On Tue, 2008-08-05 at 20:55 -0600, miltonm wrote:
> ----- Original Message Follows -----
> From: Michael Ellerman <michael@ellerman.id.au>
> To: Paul Mackerras <paulus@samba.org>
> Cc: <linuxppc-dev@ozlabs.org>, Milton Miller
> <miltonm@bga.com>, Segher Boessenkool
> <segher@kernel.crashing.org>
> Subject: [PATCH] powerpc: EOI spurious irqs during boot so
> they can be reenabled later
> Date: Wed, 6 Aug 2008 12:03:37 +1000 (EST)
>
> > In the xics code, if we receive an irq during boot that
> > hasn't been setup yet - ie. we have no reverse mapping for
> > it - we mask the irq so we don't hear from it again.
> >
> > Later on if someone request_irq()'s that irq, we'll unmask
> > it but it will still never fire. This is because we never
> > EOI'ed the irq when we originally received it - when it
> > was spurious.
> >
> > This can be reproduced trivially by banging the keyboard
> > while kexec'ing on a P5 LPAR, even though the hvc_console
> > driver request's the console irq, the console is
> > non-functional because we're receiving no console
> > interrupts.
> >
>
> which means some driver didn't disable interrupts on its
> shutdown, but I digress.
But in the case of kdump the driver never gets a chance to shutdown its
irq, but I digress too :)
> > diff --git a/arch/powerpc/platforms/pseries/xics.c
> > b/arch/powerpc/platforms/pseries/xics.c index
> > 0fc830f..4c692b2 100644 ---
> > a/arch/powerpc/platforms/pseries/xics.c +++
> > b/arch/powerpc/platforms/pseries/xics.c @@ -321,21 +321,26
> > @@ static unsigned int xics_startup(unsigned int virq)
> > return 0;
> > }
> >
> > +static void xics_eoi_hwirq_direct(unsigned int hwirq)
> > +{
> > + iosync();
> > + direct_xirr_info_set((0xff << 24) | hwirq);
> > +}
> > +
> > static void xics_eoi_direct(unsigned int virq)
> > {
> > - unsigned int irq = (unsigned int)irq_map[virq].hwirq;
> > + xics_eoi_hwirq_direct((unsigned
> > int)irq_map[virq].hwirq); +}
> >
> > +static void xics_eoi_hwirq_lpar(unsigned int hwirq)
> > +{
> > iosync();
> > - direct_xirr_info_set((0xff << 24) | irq);
> > + lpar_xirr_info_set((0xff << 24) | hwirq);
> > }
> >
> > -
> > static void xics_eoi_lpar(unsigned int virq)
> > {
> > - unsigned int irq = (unsigned int)irq_map[virq].hwirq;
> > -
> > - iosync();
> > - lpar_xirr_info_set((0xff << 24) | irq);
> > + xics_eoi_hwirq_lpar((unsigned
> > int)irq_map[virq].hwirq);
> > }
> >
> > static inline unsigned int xics_remap_irq(unsigned int
> > vec) @@ -350,9 +355,15 @@ static inline unsigned int
> > xics_remap_irq(unsigned int vec)
> > if (likely(irq != NO_IRQ))
> > return irq;
> >
> > - printk(KERN_ERR "Interrupt %u (real) is invalid,"
> > - " disabling it.\n", vec);
> > + pr_err("%s: no mapping for hwirq %u, disabling it.\n"
> > , __func__, vec); +
> > xics_mask_real_irq(vec);
> > +
> > + if (firmware_has_feature(FW_FEATURE_LPAR))
> > + xics_eoi_hwirq_lpar(vec);
> > + else
> > + xics_eoi_hwirq_direct(vec);
> > +
> > return NO_IRQ;
> > }
>
>
> I really dislike this great big if in the middle of a
> function.
> we called this function from two different call sites where
> the
> choice of which to call was based on their environment.
>
> Please move the call to eoi the hwirq to the callers, who
> know which path to take.
It's not pretty, but the alternative is worse I think:
>From 0a908825c8de6cd4c26288aba8c5b7fe3ed0a69f Mon Sep 17 00:00:00 2001
From: Michael Ellerman <michael@ellerman.id.au>
Date: Tue, 5 Aug 2008 22:34:48 +1000
Subject: [PATCH] powerpc: EOI spurious irqs during boot so they can be reenabled later
In the xics code, if we receive an irq during boot that hasn't been setup
yet - ie. we have no reverse mapping for it - we mask the irq so we don't
hear from it again.
Later on if someone request_irq()'s that irq, we'll unmask it but it will
still never fire. This is because we never EOI'ed the irq when we originally
received it - when it was spurious.
This can be reproduced trivially by banging the keyboard while kexec'ing on
a P5 LPAR, even though the hvc_console driver request's the console irq, the
console is non-functional because we're receiving no console interrupts.
So when we receive a spurious irq mask it and then EOI it.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/xics.c | 74 ++++++++++++++++++++++-----------
1 files changed, 50 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/xics.c b/arch/powerpc/platforms/pseries/xics.c
index 0fc830f..754706c 100644
--- a/arch/powerpc/platforms/pseries/xics.c
+++ b/arch/powerpc/platforms/pseries/xics.c
@@ -90,7 +90,7 @@ static inline unsigned int direct_xirr_info_get(void)
{
int cpu = smp_processor_id();
- return in_be32(&xics_per_cpu[cpu]->xirr.word);
+ return in_be32(&xics_per_cpu[cpu]->xirr.word) & 0x00ffffff;
}
static inline void direct_xirr_info_set(int value)
@@ -124,7 +124,8 @@ static inline unsigned int lpar_xirr_info_get(void)
lpar_rc = plpar_xirr(&return_value);
if (lpar_rc != H_SUCCESS)
panic(" bad return code xirr - rc = %lx \n", lpar_rc);
- return (unsigned int)return_value;
+
+ return (unsigned int)return_value & 0x00ffffff;
}
static inline void lpar_xirr_info_set(int value)
@@ -321,49 +322,74 @@ static unsigned int xics_startup(unsigned int virq)
return 0;
}
+static void xics_eoi_hwirq_direct(unsigned int hwirq)
+{
+ iosync();
+ direct_xirr_info_set((0xff << 24) | hwirq);
+}
+
static void xics_eoi_direct(unsigned int virq)
{
- unsigned int irq = (unsigned int)irq_map[virq].hwirq;
+ xics_eoi_hwirq_direct((unsigned int)irq_map[virq].hwirq);
+}
+static void xics_eoi_hwirq_lpar(unsigned int hwirq)
+{
iosync();
- direct_xirr_info_set((0xff << 24) | irq);
+ lpar_xirr_info_set((0xff << 24) | hwirq);
}
-
static void xics_eoi_lpar(unsigned int virq)
{
- unsigned int irq = (unsigned int)irq_map[virq].hwirq;
-
- iosync();
- lpar_xirr_info_set((0xff << 24) | irq);
+ xics_eoi_hwirq_lpar((unsigned int)irq_map[virq].hwirq);
}
static inline unsigned int xics_remap_irq(unsigned int vec)
{
- unsigned int irq;
-
- vec &= 0x00ffffff;
-
- if (vec == XICS_IRQ_SPURIOUS)
- return NO_IRQ;
- irq = irq_radix_revmap(xics_host, vec);
- if (likely(irq != NO_IRQ))
- return irq;
+ return irq_radix_revmap(xics_host, vec);
+}
- printk(KERN_ERR "Interrupt %u (real) is invalid,"
- " disabling it.\n", vec);
- xics_mask_real_irq(vec);
- return NO_IRQ;
+static void xics_report_bad_irq(unsigned int hwirq)
+{
+ pr_err("%s: no mapping for hwirq %u, disabling it.\n", __func__, hwirq);
}
static unsigned int xics_get_irq_direct(void)
{
- return xics_remap_irq(direct_xirr_info_get());
+ unsigned int virq, hwirq;
+
+ hwirq = direct_xirr_info_get();
+ if (hwirq == XICS_IRQ_SPURIOUS)
+ return NO_IRQ;
+
+ virq = xics_remap_irq(hwirq);
+
+ if (unlikely(virq == NO_IRQ)) {
+ xics_report_bad_irq(hwirq);
+ xics_mask_real_irq(hwirq);
+ xics_eoi_hwirq_direct(hwirq);
+ }
+
+ return virq;
}
static unsigned int xics_get_irq_lpar(void)
{
- return xics_remap_irq(lpar_xirr_info_get());
+ unsigned int virq, hwirq;
+
+ hwirq = lpar_xirr_info_get();
+ if (hwirq == XICS_IRQ_SPURIOUS)
+ return NO_IRQ;
+
+ virq = xics_remap_irq(hwirq);
+
+ if (unlikely(virq == NO_IRQ)) {
+ xics_report_bad_irq(hwirq);
+ xics_mask_real_irq(hwirq);
+ xics_eoi_hwirq_lpar(hwirq);
+ }
+
+ return virq;
}
#ifdef CONFIG_SMP
--
1.5.5
^ permalink raw reply related
* libfdt: Implement fdt_get_property_namelen() and fdt_getprop_namelen()
From: David Gibson @ 2008-08-06 4:50 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
As well as fdt_subnode_offset(), libfdt includes an
fdt_subnode_offset_namelen() function that takes the subnode name to
look up not as a NUL-terminated string, but as a string with an
explicit length. This can be useful when the caller has the name as
part of a longer string, such as a full path.
However, we don't have corresponding 'namelen' versions for
fdt_get_property() and fdt_getprop(). There are less obvious use
cases for these variants on property names, but there are
circumstances where they can be useful e.g. looking up property names
which need to be parsed from a longer string buffer such as user input
or a configuration file, or looking up an alias in a path with
IEEE1275 style aliases.
So, since it's very easy to implement such variants, this patch does
so. The original NUL-terminated variants are, of course, implemented
in terms of the namelen versions.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Index: dtc/libfdt/fdt_ro.c
===================================================================
--- dtc.orig/libfdt/fdt_ro.c 2008-08-05 14:12:17.000000000 +1000
+++ dtc/libfdt/fdt_ro.c 2008-08-05 14:39:19.000000000 +1000
@@ -80,6 +80,14 @@ const char *fdt_string(const void *fdt,
return (const char *)fdt + fdt_off_dt_strings(fdt) + stroffset;
}
+static int _fdt_string_eq(const void *fdt, int stroffset,
+ const char *s, int len)
+{
+ const char *p = fdt_string(fdt, stroffset);
+
+ return (strlen(p) == len) && (memcmp(p, s, len) == 0);
+}
+
int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size)
{
FDT_CHECK_HEADER(fdt);
@@ -175,9 +183,10 @@ const char *fdt_get_name(const void *fdt
return NULL;
}
-const struct fdt_property *fdt_get_property(const void *fdt,
- int nodeoffset,
- const char *name, int *lenp)
+const struct fdt_property *fdt_get_property_namelen(const void *fdt,
+ int nodeoffset,
+ const char *name,
+ int namelen, int *lenp)
{
uint32_t tag;
const struct fdt_property *prop;
@@ -210,7 +219,7 @@ const struct fdt_property *fdt_get_prope
if (! prop)
goto fail;
namestroff = fdt32_to_cpu(prop->nameoff);
- if (strcmp(fdt_string(fdt, namestroff), name) == 0) {
+ if (_fdt_string_eq(fdt, namestroff, name, namelen)) {
/* Found it! */
int len = fdt32_to_cpu(prop->len);
prop = fdt_offset_ptr(fdt, offset,
@@ -238,18 +247,32 @@ const struct fdt_property *fdt_get_prope
return NULL;
}
-const void *fdt_getprop(const void *fdt, int nodeoffset,
- const char *name, int *lenp)
+const struct fdt_property *fdt_get_property(const void *fdt,
+ int nodeoffset,
+ const char *name, int *lenp)
+{
+ return fdt_get_property_namelen(fdt, nodeoffset, name,
+ strlen(name), lenp);
+}
+
+const void *fdt_getprop_namelen(const void *fdt, int nodeoffset,
+ const char *name, int namelen, int *lenp)
{
const struct fdt_property *prop;
- prop = fdt_get_property(fdt, nodeoffset, name, lenp);
+ prop = fdt_get_property_namelen(fdt, nodeoffset, name, namelen, lenp);
if (! prop)
return NULL;
return prop->data;
}
+const void *fdt_getprop(const void *fdt, int nodeoffset,
+ const char *name, int *lenp)
+{
+ return fdt_getprop_namelen(fdt, nodeoffset, name, strlen(name), lenp);
+}
+
uint32_t fdt_get_phandle(const void *fdt, int nodeoffset)
{
const uint32_t *php;
Index: dtc/libfdt/libfdt.h
===================================================================
--- dtc.orig/libfdt/libfdt.h 2008-08-05 14:29:15.000000000 +1000
+++ dtc/libfdt/libfdt.h 2008-08-05 14:40:58.000000000 +1000
@@ -343,6 +343,22 @@ int fdt_path_offset(const void *fdt, con
const char *fdt_get_name(const void *fdt, int nodeoffset, int *lenp);
/**
+ * fdt_get_property_namelen - find a property based on substring
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to find
+ * @name: name of the property to find
+ * @namelen: number of characters of name to consider
+ * @lenp: pointer to an integer variable (will be overwritten) or NULL
+ *
+ * Identical to fdt_get_property_namelen(), but only examine the first
+ * namelen characters of name for matching the property name.
+ */
+const struct fdt_property *fdt_get_property_namelen(const void *fdt,
+ int nodeoffset,
+ const char *name,
+ int namelen, int *lenp);
+
+/**
* fdt_get_property - find a given property in a given node
* @fdt: pointer to the device tree blob
* @nodeoffset: offset of the node whose property to find
@@ -380,6 +396,20 @@ static inline struct fdt_property *fdt_g
}
/**
+ * fdt_getprop_namelen - get property value based on substring
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to find
+ * @name: name of the property to find
+ * @namelen: number of characters of name to consider
+ * @lenp: pointer to an integer variable (will be overwritten) or NULL
+ *
+ * Identical to fdt_getprop(), but only examine the first namelen
+ * characters of name for matching the property name.
+ */
+const void *fdt_getprop_namelen(const void *fdt, int nodeoffset,
+ const char *name, int namelen, int *lenp);
+
+/**
* fdt_getprop - retrieve the value of a given property
* @fdt: pointer to the device tree blob
* @nodeoffset: offset of the node whose property to find
--
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 add immr alias 1/4] powerpc: Teach get_immrbase to use immr alias if it exists.
From: Stephen Rothwell @ 2008-08-06 3:41 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev
In-Reply-To: <1217967220-30557-1-git-send-email-jrigby@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 578 bytes --]
Hi John,
[From further in the discussion, this may no longer be relevant ...]
On Tue, 5 Aug 2008 14:13:37 -0600 John Rigby <jrigby@freescale.com> wrote:
>
> - soc = of_find_node_by_type(NULL, "soc");
> + /*
> + * First look for an immr alias
> + */
> + np = of_find_node_by_name(NULL, "/aliases");
> + if (np) {
> + path = of_get_property(np, "immr", NULL);
> + if (path)
> + soc = of_find_node_by_name(NULL, path);
of_node_put(np);
> + }
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: EOI spurious irqs during boot so they can be reenabled later
From: miltonm @ 2008-08-06 2:55 UTC (permalink / raw)
To: Michael Ellerman, Paul Mackerras; +Cc: linuxppc-dev, Milton Miller
----- Original Message Follows -----
From: Michael Ellerman <michael@ellerman.id.au>
To: Paul Mackerras <paulus@samba.org>
Cc: <linuxppc-dev@ozlabs.org>, Milton Miller
<miltonm@bga.com>, Segher Boessenkool
<segher@kernel.crashing.org>
Subject: [PATCH] powerpc: EOI spurious irqs during boot so
they can be reenabled later
Date: Wed, 6 Aug 2008 12:03:37 +1000 (EST)
> In the xics code, if we receive an irq during boot that
> hasn't been setup yet - ie. we have no reverse mapping for
> it - we mask the irq so we don't hear from it again.
>
> Later on if someone request_irq()'s that irq, we'll unmask
> it but it will still never fire. This is because we never
> EOI'ed the irq when we originally received it - when it
> was spurious.
>
> This can be reproduced trivially by banging the keyboard
> while kexec'ing on a P5 LPAR, even though the hvc_console
> driver request's the console irq, the console is
> non-functional because we're receiving no console
> interrupts.
>
which means some driver didn't disable interrupts on its
shutdown, but I digress.
> So when we receive a spurious irq mask it and then EOI it.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> arch/powerpc/platforms/pseries/xics.c | 29
> ++++++++++++++++++++---------
> 1 files changed, 20 insertions(+), 9 deletions(-)
>
>
> Updated to mask then EOI, thanks to Segher for whacking me
> with the clue stick.
>
Actually, just sending the EOI would likely result in the
exact same interrupt comming back, as the mask will set
a bit near the source but the interrupt would have already
been missed. (most irqs in xics systems are level).
Thanks to segher for noticing the order. However...
> diff --git a/arch/powerpc/platforms/pseries/xics.c
> b/arch/powerpc/platforms/pseries/xics.c index
> 0fc830f..4c692b2 100644 ---
> a/arch/powerpc/platforms/pseries/xics.c +++
> b/arch/powerpc/platforms/pseries/xics.c @@ -321,21 +321,26
> @@ static unsigned int xics_startup(unsigned int virq)
> return 0;
> }
>
> +static void xics_eoi_hwirq_direct(unsigned int hwirq)
> +{
> + iosync();
> + direct_xirr_info_set((0xff << 24) | hwirq);
> +}
> +
> static void xics_eoi_direct(unsigned int virq)
> {
> - unsigned int irq = (unsigned int)irq_map[virq].hwirq;
> + xics_eoi_hwirq_direct((unsigned
> int)irq_map[virq].hwirq); +}
>
> +static void xics_eoi_hwirq_lpar(unsigned int hwirq)
> +{
> iosync();
> - direct_xirr_info_set((0xff << 24) | irq);
> + lpar_xirr_info_set((0xff << 24) | hwirq);
> }
>
> -
> static void xics_eoi_lpar(unsigned int virq)
> {
> - unsigned int irq = (unsigned int)irq_map[virq].hwirq;
> -
> - iosync();
> - lpar_xirr_info_set((0xff << 24) | irq);
> + xics_eoi_hwirq_lpar((unsigned
> int)irq_map[virq].hwirq);
> }
>
> static inline unsigned int xics_remap_irq(unsigned int
> vec) @@ -350,9 +355,15 @@ static inline unsigned int
> xics_remap_irq(unsigned int vec)
> if (likely(irq != NO_IRQ))
> return irq;
>
> - printk(KERN_ERR "Interrupt %u (real) is invalid,"
> - " disabling it.\n", vec);
> + pr_err("%s: no mapping for hwirq %u, disabling it.\n"
> , __func__, vec); +
> xics_mask_real_irq(vec);
> +
> + if (firmware_has_feature(FW_FEATURE_LPAR))
> + xics_eoi_hwirq_lpar(vec);
> + else
> + xics_eoi_hwirq_direct(vec);
> +
> return NO_IRQ;
> }
I really dislike this great big if in the middle of a
function.
we called this function from two different call sites where
the
choice of which to call was based on their environment.
Please move the call to eoi the hwirq to the callers, who
know
which path to take.
I guess it might make sense to put the printk in a
mask_invalid_real_irq wrapper, and then perhaps just
duplicate
the small remaining code? Or split the return of spurious
from NO_IRQ (ie should spurious be NO_IRQ_IGNORE?)
>
> --
> 1.5.5
>
^ permalink raw reply
* Re: inline assembly & r0 SOS
From: Kevin Diggs @ 2008-08-06 2:31 UTC (permalink / raw)
To: Jeremy Kerr; +Cc: linuxppc-dev
In-Reply-To: <200808061041.50577.jk@ozlabs.org>
Jeremy Kerr wrote:
> Hi Kevin,
>
>
>> /*
>> * Turn r3 (range) into a rotate count for the selected
>>range. * 0 -> 23, 1 -> 31
>> */
>> __asm__ __volatile__ ( "slwi %0,%0,3\n"
>> "addi %0,%0,23\n"
>> "rlwnm %0,%1,%0,30,31\n":
>> "=r"(ret):
>> "r"(config),"0"(range)
>> );
>
>
> Wouldn't this be much simpler in plain C?
>
I just knew someone was gonna try to rain on my rlwnm'in fun parade!
Wonder if the C code can be made to compile down to 3 instructions?
> However, if you really do need to do this in inline asm, you can use
> the "b" modifier rather than "r" to avoid using r0.
>
Oh cool! Thanks! You to Ben!
> Cheers,
>
>
> Jeremy
>
kevin
^ permalink raw reply
* [PATCH] powerpc: EOI spurious irqs during boot so they can be reenabled later
From: Michael Ellerman @ 2008-08-06 2:03 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, Milton Miller
In the xics code, if we receive an irq during boot that hasn't been setup
yet - ie. we have no reverse mapping for it - we mask the irq so we don't
hear from it again.
Later on if someone request_irq()'s that irq, we'll unmask it but it will
still never fire. This is because we never EOI'ed the irq when we originally
received it - when it was spurious.
This can be reproduced trivially by banging the keyboard while kexec'ing on
a P5 LPAR, even though the hvc_console driver request's the console irq, the
console is non-functional because we're receiving no console interrupts.
So when we receive a spurious irq mask it and then EOI it.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/platforms/pseries/xics.c | 29 ++++++++++++++++++++---------
1 files changed, 20 insertions(+), 9 deletions(-)
Updated to mask then EOI, thanks to Segher for whacking me with the clue stick.
diff --git a/arch/powerpc/platforms/pseries/xics.c b/arch/powerpc/platforms/pseries/xics.c
index 0fc830f..4c692b2 100644
--- a/arch/powerpc/platforms/pseries/xics.c
+++ b/arch/powerpc/platforms/pseries/xics.c
@@ -321,21 +321,26 @@ static unsigned int xics_startup(unsigned int virq)
return 0;
}
+static void xics_eoi_hwirq_direct(unsigned int hwirq)
+{
+ iosync();
+ direct_xirr_info_set((0xff << 24) | hwirq);
+}
+
static void xics_eoi_direct(unsigned int virq)
{
- unsigned int irq = (unsigned int)irq_map[virq].hwirq;
+ xics_eoi_hwirq_direct((unsigned int)irq_map[virq].hwirq);
+}
+static void xics_eoi_hwirq_lpar(unsigned int hwirq)
+{
iosync();
- direct_xirr_info_set((0xff << 24) | irq);
+ lpar_xirr_info_set((0xff << 24) | hwirq);
}
-
static void xics_eoi_lpar(unsigned int virq)
{
- unsigned int irq = (unsigned int)irq_map[virq].hwirq;
-
- iosync();
- lpar_xirr_info_set((0xff << 24) | irq);
+ xics_eoi_hwirq_lpar((unsigned int)irq_map[virq].hwirq);
}
static inline unsigned int xics_remap_irq(unsigned int vec)
@@ -350,9 +355,15 @@ static inline unsigned int xics_remap_irq(unsigned int vec)
if (likely(irq != NO_IRQ))
return irq;
- printk(KERN_ERR "Interrupt %u (real) is invalid,"
- " disabling it.\n", vec);
+ pr_err("%s: no mapping for hwirq %u, disabling it.\n", __func__, vec);
+
xics_mask_real_irq(vec);
+
+ if (firmware_has_feature(FW_FEATURE_LPAR))
+ xics_eoi_hwirq_lpar(vec);
+ else
+ xics_eoi_hwirq_direct(vec);
+
return NO_IRQ;
}
--
1.5.5
^ permalink raw reply related
* Re: to schedule() or not to schedule() ?
From: Kevin Diggs @ 2008-08-06 1:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1217977256.7593.2.camel@localhost>
Michael Ellerman wrote:
> On Tue, 2008-08-05 at 12:26 -0700, Kevin Diggs wrote:
>
>>Chris Friesen wrote:
>>
>>>Kevin Diggs wrote:
>
>
>>>> I have the following near the top of my cpufreq driver target
>>>>routine:
>>>>
>>>>while(test_and_set_bit(cf750gxmCfgChangeBit,&cf750gxvStateBits)) {
>>>> /*
>>>> * Someone mucking with our cfg? (I hope it is ok to call
>>>> * schedule() here! - truth is I have no idea what I am doing
>>>> * ... my reasoning is I want to yeild the cpu so whoever is
>>>> * mucking around can finish)
>>>> */
>>>> schedule();
>>>>}
>>>>
>>>>This is to prevent bad things from happening if someone is trying to
>>>>change a parameter for the driver via sysfs while the target routine
>>>>is running. Fortunately, because I had a bug where this bit was not
>>>>getting cleared on one of the paths through the target routine ... I
>>>>now know it is not safe to call schedule (it got stuck in there -
>>>>knocked out my adb keyboard! - (I think target is called from a timer
>>>>that the governor sets up ... interrupt context?)).
>>>
>>>
>>>Is the issue that someone may be in the middle of a multi-stage
>>>procedure, and you've woken up partway through?
>>>
>>>If so, what about simply rescheduling the timer for some short time in
>>>the future and aborting the current call?
>
>
>>Chris,
>>
>> Thanks for taking the time to reply. The parameter in question modifies
>>the frequency table. It is used several times in the target routine.
>>I've addressed the issue by making a local copy of the frequency table
>>upon entry to the target routine and use that while there. I don't care
>>who wins the race.
>
>
> How are you copying the table? Is it an atomic copy? Otherwise you could
> just end up copying the table while it's being updated, and you get a
> copy of the partially updated table.
>
> Don't you just need a spinlock?
>
> cheers
>
In the initialization routine I create 2 tables. One is a table with all
the frequencies. The other has just the min and max. The parameter just
changes a pointer to point to one table or the other. The above
addressing of the issue should really say "a local copy of the pointer
to the frequency table".
Thanks for the reply!
For the purpose of learning, there is no direct, correct way to yield
the cpu when in a timer fired routine, right?
kevin
^ permalink raw reply
* Re: [PATCH] powerpc: EOI spurious irqs during boot so they can be reenabled later
From: Michael Ellerman @ 2008-08-06 1:58 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, Paul Mackerras, Milton Miller
In-Reply-To: <02B3273A-55F2-42A3-AB17-8B90FCE27961@kernel.crashing.org>
[-- Attachment #1: Type: text/plain, Size: 793 bytes --]
On Tue, 2008-08-05 at 18:48 +0200, Segher Boessenkool wrote:
> > So when we receive a spurious irq, EOI it, and then mask it.
>
> What happens when a new IRQ arrives on the interrupt controller
> between these EOI and mask calls? Should you instead first mask
> it, and only then EOI it? Or doesn't that work on XICS?
Yeah right, in which case we'd have not EOI'ed that one and we're back
at square one.
It seems to work with a mask then EOI, so unless Milton says otherwise
I'll do it that way - new patch coming.
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: inline assembly & r0 SOS
From: Jeremy Kerr @ 2008-08-06 0:41 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Kevin Diggs
In-Reply-To: <4898EE60.9080206@hypersurf.com>
Hi Kevin,
> /*
> * Turn r3 (range) into a rotate count for the selected
> range. * 0 -> 23, 1 -> 31
> */
> __asm__ __volatile__ ( "slwi %0,%0,3\n"
> "addi %0,%0,23\n"
> "rlwnm %0,%1,%0,30,31\n":
> "=r"(ret):
> "r"(config),"0"(range)
> );
Wouldn't this be much simpler in plain C?
However, if you really do need to do this in inline asm, you can use
the "b" modifier rather than "r" to avoid using r0.
Cheers,
Jeremy
^ permalink raw reply
* Re: inline assembly & r0 SOS
From: Benjamin Herrenschmidt @ 2008-08-06 0:35 UTC (permalink / raw)
To: Kevin Diggs; +Cc: linuxppc-dev
In-Reply-To: <4898EE60.9080206@hypersurf.com>
On Tue, 2008-08-05 at 17:20 -0700, Kevin Diggs wrote:
> Hi,
>
> thats bad right? Because the "addi 0, 0, 23" will not work as expected
> because of the "special property" of r0. FYI: The first three lines
> after the "#APP" are from a similar function get_PLL_ratio().
>
> Is there a way to blacklist r0?
Yes. Use "b" instead of "r" in the constraints to force the compiler
to avoid r0 when assigning a register.
Cheers,
Ben.
^ permalink raw reply
* inline assembly & r0 SOS
From: Kevin Diggs @ 2008-08-06 0:20 UTC (permalink / raw)
To: linuxppc-dev
Hi,
If I have:
inline unsigned int get_PLL_range(unsigned int range, unsigned int
config)
{
unsigned int ret;
/*
* Turn r3 (range) into a rotate count for the selected range.
* 0 -> 23, 1 -> 31
*/
__asm__ __volatile__ ( "slwi %0,%0,3\n"
"addi %0,%0,23\n"
"rlwnm %0,%1,%0,30,31\n":
"=r"(ret):
"r"(config),"0"(range)
);
return ret;
}
in a header and the resultant code generated is:
.L58:
li 0,1 # ratio,
mr 29,0 # ret, ratio
#APP
slwi 29,29,3 # ret
addi 29,29,21 # ret
rlwnm 29,28,29,27,31 # ret, ret
slwi 0,0,3 # ret
addi 0,0,23 # ret
rlwnm 0,28,0,30,31 # ret, ret
#NO_APP
thats bad right? Because the "addi 0, 0, 23" will not work as expected
because of the "special property" of r0. FYI: The first three lines
after the "#APP" are from a similar function get_PLL_ratio().
Is there a way to blacklist r0?
kevin
^ permalink raw reply
* Re: [PATCH add immr alias 2/4] powerpc: 5121: Add immr alias to MPC5121 ADS device tree.
From: Anton Vorontsov @ 2008-08-05 23:46 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, John Rigby
In-Reply-To: <4898C3F0.8060305@freescale.com>
On Tue, Aug 05, 2008 at 04:19:44PM -0500, Scott Wood wrote:
> Grant Likely wrote:
>> But finding nodes that meet a criteria *is* what compatible is for and
>> there is precedence for it. All u-boot platforms are finding the node
>> by path right now, and so all of them need to be changed. Changing
>> them to find by compatible that is set per-board or per-SoC makes
>> complete sense to me.
>
> It is ridiculous to have to duplicate code (or create a table, or
> whatever) just so it can search for mpc8536-foo, mpc8544-foo,
> mpc8548-foo, etc -- and in the case of the SoC, it's *not* fully
> compatible, so we *can't* pick one as the "default" -- but it's
> compatible for the purposes of the code in question.
>
> I figured an alias would attract fewer flames than a compatible of
> "fsl,immr" (though I'm fine with it -- it's specifying compatibility of
> device tree binding, not of the hardware).
>
> And no, they're not all finding it by path now -- there's a lot of use
> of device_type "soc", which is what we're trying to avoid by introducing
> this alias. The bootwrapper is also affected.
FWIW, recent u-boot also looks for "fsl,soc" compatible entry.
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: PS3 early lock-up
From: Geoff Levand @ 2008-08-05 23:24 UTC (permalink / raw)
To: benh; +Cc: Geert Uytterhoeven, Linux/PPC Development
In-Reply-To: <1217932117.24157.185.camel@pasglop>
Benjamin Herrenschmidt wrote:
>> arch/powerpc/platfroms/ps3/htab.c:ps3_hpte_updatepp() uses `htab[slot]=
.v'.
>=20
> Ok, that leaves us with 2 options:
>=20
> - Change ps3_hpte_updatepp() to not read from the hash table via that
> mapping (ie, do you have an LV1 call to read an HPTE ? Do you measure
> any significant performance loss using that instead ? updatepp shouldn'=
t
> be something called -that- often).
Yes, we have lv1_read_htab_entries(). Mokuno-san started some work to
convert to using it and removing the htab mapping:
http://git.kernel.org/?p=3Dlinux/kernel/git/geoff/ps3-linux-patches.git=
;a=3Dblob;f=3Dps3-wip/ps3-htab-rework.diff;hb=3DHEAD
Unfortunately, this week Mokuno-san is on holiday, and I am busy preparin=
g
for SIGGRAPH (next week).
> - Add a way to setup HPTEs using 3 PPP bits. I'm not going to implemen=
t
> that for the main hash code just yet though (the assembly) but it might
> be possible to implement it specifically for mappings bolted. That
> means it would only work when the mapping is done early but on PS3, we
> know that the hash table is always mapped early.
>=20
> The later would be a matter of taking my =EF=BB=BFhtab_convert_pte_flag=
s() function
> and making it capable, when _PAGE_USER is _not_ set and _PAGE_RW is not
> set neither, to set PPP to 110.
>=20
> You could do that by adding:
>=20
> if (!(pteflags & (_PAGE_USER | _PAGE_RW)))
> rflags |=3D (1 << 1) | (1 << 63);
>=20
> Dbl check that the resulting mapping isn't accessible to user space tho=
ugh.
If we can't remove the htab mapping with lv1_read_htab_entries(), I'll
look into this way.
-Geoff
^ permalink raw reply
* [PATCH 4/4] powerpc: Convert the MPIC MSI code to use msi_bitmap
From: Michael Ellerman @ 2008-08-05 23:10 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <b3997f77b701a0886df6eb3a5e4446d4dc538483.1217977742.git.michael@ellerman.id.au>
This effects the U3 MSI code as well as the PASEMI MSI code. We keep
some of the MPIC routines as helpers, and also the U3 best-guess
reservation logic. The rest is replaced by the generic code.
And a few printk format changes due to hwirq type change.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/include/asm/mpic.h | 4 +-
arch/powerpc/sysdev/mpic.h | 2 -
arch/powerpc/sysdev/mpic_msi.c | 123 +++++----------------------------
arch/powerpc/sysdev/mpic_pasemi_msi.c | 24 ++++---
arch/powerpc/sysdev/mpic_u3msi.c | 22 +++---
5 files changed, 44 insertions(+), 131 deletions(-)
diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index fe566a3..34d9ac4 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -5,6 +5,7 @@
#include <linux/irq.h>
#include <linux/sysdev.h>
#include <asm/dcr.h>
+#include <asm/msi_bitmap.h>
/*
* Global registers
@@ -301,8 +302,7 @@ struct mpic
#endif
#ifdef CONFIG_PCI_MSI
- spinlock_t bitmap_lock;
- unsigned long *hwirq_bitmap;
+ struct msi_bitmap msi_bitmap;
#endif
#ifdef CONFIG_MPIC_BROKEN_REGREAD
diff --git a/arch/powerpc/sysdev/mpic.h b/arch/powerpc/sysdev/mpic.h
index fbf8a26..6209c62 100644
--- a/arch/powerpc/sysdev/mpic.h
+++ b/arch/powerpc/sysdev/mpic.h
@@ -14,8 +14,6 @@
#ifdef CONFIG_PCI_MSI
extern void mpic_msi_reserve_hwirq(struct mpic *mpic, irq_hw_number_t hwirq);
extern int mpic_msi_init_allocator(struct mpic *mpic);
-extern irq_hw_number_t mpic_msi_alloc_hwirqs(struct mpic *mpic, int num);
-extern void mpic_msi_free_hwirqs(struct mpic *mpic, int offset, int num);
extern int mpic_u3msi_init(struct mpic *mpic);
extern int mpic_pasemi_msi_init(struct mpic *mpic);
#else
diff --git a/arch/powerpc/sysdev/mpic_msi.c b/arch/powerpc/sysdev/mpic_msi.c
index de3e5e8..1d44eee 100644
--- a/arch/powerpc/sysdev/mpic_msi.c
+++ b/arch/powerpc/sysdev/mpic_msi.c
@@ -15,59 +15,17 @@
#include <asm/prom.h>
#include <asm/hw_irq.h>
#include <asm/ppc-pci.h>
+#include <asm/msi_bitmap.h>
#include <sysdev/mpic.h>
-static void __mpic_msi_reserve_hwirq(struct mpic *mpic, irq_hw_number_t hwirq)
-{
- pr_debug("mpic: reserving hwirq 0x%lx\n", hwirq);
- bitmap_allocate_region(mpic->hwirq_bitmap, hwirq, 0);
-}
-
void mpic_msi_reserve_hwirq(struct mpic *mpic, irq_hw_number_t hwirq)
{
- unsigned long flags;
-
/* The mpic calls this even when there is no allocator setup */
- if (!mpic->hwirq_bitmap)
+ if (!mpic->msi_bitmap.bitmap)
return;
- spin_lock_irqsave(&mpic->bitmap_lock, flags);
- __mpic_msi_reserve_hwirq(mpic, hwirq);
- spin_unlock_irqrestore(&mpic->bitmap_lock, flags);
-}
-
-irq_hw_number_t mpic_msi_alloc_hwirqs(struct mpic *mpic, int num)
-{
- unsigned long flags;
- int offset, order = get_count_order(num);
-
- spin_lock_irqsave(&mpic->bitmap_lock, flags);
- /*
- * This is fast, but stricter than we need. We might want to add
- * a fallback routine which does a linear search with no alignment.
- */
- offset = bitmap_find_free_region(mpic->hwirq_bitmap, mpic->irq_count,
- order);
- spin_unlock_irqrestore(&mpic->bitmap_lock, flags);
-
- pr_debug("mpic: allocated 0x%x (2^%d) at offset 0x%x\n",
- num, order, offset);
-
- return offset;
-}
-
-void mpic_msi_free_hwirqs(struct mpic *mpic, int offset, int num)
-{
- unsigned long flags;
- int order = get_count_order(num);
-
- pr_debug("mpic: freeing 0x%x (2^%d) at offset 0x%x\n",
- num, order, offset);
-
- spin_lock_irqsave(&mpic->bitmap_lock, flags);
- bitmap_release_region(mpic->hwirq_bitmap, offset, order);
- spin_unlock_irqrestore(&mpic->bitmap_lock, flags);
+ msi_bitmap_reserve_hwirq(&mpic->msi_bitmap, hwirq);
}
#ifdef CONFIG_MPIC_U3_HT_IRQS
@@ -83,13 +41,13 @@ static int mpic_msi_reserve_u3_hwirqs(struct mpic *mpic)
/* Reserve source numbers we know are reserved in the HW */
for (i = 0; i < 8; i++)
- __mpic_msi_reserve_hwirq(mpic, i);
+ msi_bitmap_reserve_hwirq(&mpic->msi_bitmap, i);
for (i = 42; i < 46; i++)
- __mpic_msi_reserve_hwirq(mpic, i);
+ msi_bitmap_reserve_hwirq(&mpic->msi_bitmap, i);
for (i = 100; i < 105; i++)
- __mpic_msi_reserve_hwirq(mpic, i);
+ msi_bitmap_reserve_hwirq(&mpic->msi_bitmap, i);
np = NULL;
while ((np = of_find_all_nodes(np))) {
@@ -99,7 +57,7 @@ static int mpic_msi_reserve_u3_hwirqs(struct mpic *mpic)
while (of_irq_map_one(np, index++, &oirq) == 0) {
ops->xlate(mpic->irqhost, NULL, oirq.specifier,
oirq.size, &hwirq, &flags);
- __mpic_msi_reserve_hwirq(mpic, hwirq);
+ msi_bitmap_reserve_hwirq(&mpic->msi_bitmap, hwirq);
}
}
@@ -112,70 +70,25 @@ static int mpic_msi_reserve_u3_hwirqs(struct mpic *mpic)
}
#endif
-static int mpic_msi_reserve_dt_hwirqs(struct mpic *mpic)
-{
- int i, len;
- const u32 *p;
-
- p = of_get_property(mpic->irqhost->of_node,
- "msi-available-ranges", &len);
- if (!p) {
- pr_debug("mpic: no msi-available-ranges property found on %s\n",
- mpic->irqhost->of_node->full_name);
- return -ENODEV;
- }
-
- if (len % 8 != 0) {
- printk(KERN_WARNING "mpic: Malformed msi-available-ranges "
- "property on %s\n", mpic->irqhost->of_node->full_name);
- return -EINVAL;
- }
-
- bitmap_allocate_region(mpic->hwirq_bitmap, 0,
- get_count_order(mpic->irq_count));
-
- /* Format is: (<u32 start> <u32 count>)+ */
- len /= sizeof(u32);
- for (i = 0; i < len / 2; i++, p += 2)
- mpic_msi_free_hwirqs(mpic, *p, *(p + 1));
-
- return 0;
-}
-
int mpic_msi_init_allocator(struct mpic *mpic)
{
- int rc, size;
-
- BUG_ON(mpic->hwirq_bitmap);
- spin_lock_init(&mpic->bitmap_lock);
+ int rc;
- size = BITS_TO_LONGS(mpic->irq_count) * sizeof(long);
- pr_debug("mpic: allocator bitmap size is 0x%x bytes\n", size);
+ rc = msi_bitmap_alloc(&mpic->msi_bitmap, mpic->irq_count,
+ mpic->irqhost->of_node);
+ if (rc)
+ return rc;
- mpic->hwirq_bitmap = alloc_maybe_bootmem(size, GFP_KERNEL);
-
- if (!mpic->hwirq_bitmap) {
- pr_debug("mpic: ENOMEM allocating allocator bitmap!\n");
- return -ENOMEM;
- }
-
- memset(mpic->hwirq_bitmap, 0, size);
-
- rc = mpic_msi_reserve_dt_hwirqs(mpic);
- if (rc) {
+ rc = msi_bitmap_reserve_dt_hwirqs(&mpic->msi_bitmap);
+ if (rc > 0) {
if (mpic->flags & MPIC_U3_HT_IRQS)
rc = mpic_msi_reserve_u3_hwirqs(mpic);
- if (rc)
- goto out_free;
+ if (rc) {
+ msi_bitmap_free(&mpic->msi_bitmap);
+ return rc;
+ }
}
return 0;
-
- out_free:
- if (mem_init_done)
- kfree(mpic->hwirq_bitmap);
-
- mpic->hwirq_bitmap = NULL;
- return rc;
}
diff --git a/arch/powerpc/sysdev/mpic_pasemi_msi.c b/arch/powerpc/sysdev/mpic_pasemi_msi.c
index 68aff60..656cb77 100644
--- a/arch/powerpc/sysdev/mpic_pasemi_msi.c
+++ b/arch/powerpc/sysdev/mpic_pasemi_msi.c
@@ -22,6 +22,7 @@
#include <asm/prom.h>
#include <asm/hw_irq.h>
#include <asm/ppc-pci.h>
+#include <asm/msi_bitmap.h>
#include "mpic.h"
@@ -81,8 +82,8 @@ static void pasemi_msi_teardown_msi_irqs(struct pci_dev *pdev)
continue;
set_irq_msi(entry->irq, NULL);
- mpic_msi_free_hwirqs(msi_mpic, virq_to_hw(entry->irq),
- ALLOC_CHUNK);
+ msi_bitmap_free_hwirqs(&msi_mpic->msi_bitmap,
+ virq_to_hw(entry->irq), ALLOC_CHUNK);
irq_dispose_mapping(entry->irq);
}
@@ -91,11 +92,10 @@ static void pasemi_msi_teardown_msi_irqs(struct pci_dev *pdev)
static int pasemi_msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
{
- irq_hw_number_t hwirq;
unsigned int virq;
struct msi_desc *entry;
struct msi_msg msg;
- int ret;
+ int hwirq;
pr_debug("pasemi_msi_setup_msi_irqs, pdev %p nvec %d type %d\n",
pdev, nvec, type);
@@ -109,17 +109,19 @@ static int pasemi_msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
* few MSIs for someone, but restrictions will apply to how the
* sources can be changed independently.
*/
- ret = mpic_msi_alloc_hwirqs(msi_mpic, ALLOC_CHUNK);
- hwirq = ret;
- if (ret < 0) {
+ hwirq = msi_bitmap_alloc_hwirqs(&msi_mpic->msi_bitmap,
+ ALLOC_CHUNK);
+ if (hwirq < 0) {
pr_debug("pasemi_msi: failed allocating hwirq\n");
return hwirq;
}
virq = irq_create_mapping(msi_mpic->irqhost, hwirq);
if (virq == NO_IRQ) {
- pr_debug("pasemi_msi: failed mapping hwirq 0x%lx\n", hwirq);
- mpic_msi_free_hwirqs(msi_mpic, hwirq, ALLOC_CHUNK);
+ pr_debug("pasemi_msi: failed mapping hwirq 0x%x\n",
+ hwirq);
+ msi_bitmap_free_hwirqs(&msi_mpic->msi_bitmap, hwirq,
+ ALLOC_CHUNK);
return -ENOSPC;
}
@@ -133,8 +135,8 @@ static int pasemi_msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
set_irq_chip(virq, &mpic_pasemi_msi_chip);
set_irq_type(virq, IRQ_TYPE_EDGE_RISING);
- pr_debug("pasemi_msi: allocated virq 0x%x (hw 0x%lx) addr 0x%x\n",
- virq, hwirq, msg.address_lo);
+ pr_debug("pasemi_msi: allocated virq 0x%x (hw 0x%x) " \
+ "addr 0x%x\n", virq, hwirq, msg.address_lo);
/* Likewise, the device writes [0...511] into the target
* register to generate MSI [512...1023]
diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
index 6e2f868..0a8f5a9 100644
--- a/arch/powerpc/sysdev/mpic_u3msi.c
+++ b/arch/powerpc/sysdev/mpic_u3msi.c
@@ -16,6 +16,7 @@
#include <asm/prom.h>
#include <asm/hw_irq.h>
#include <asm/ppc-pci.h>
+#include <asm/msi_bitmap.h>
#include "mpic.h"
@@ -101,7 +102,8 @@ static void u3msi_teardown_msi_irqs(struct pci_dev *pdev)
continue;
set_irq_msi(entry->irq, NULL);
- mpic_msi_free_hwirqs(msi_mpic, virq_to_hw(entry->irq), 1);
+ msi_bitmap_free_hwirqs(&msi_mpic->msi_bitmap,
+ virq_to_hw(entry->irq), 1);
irq_dispose_mapping(entry->irq);
}
@@ -110,29 +112,27 @@ static void u3msi_teardown_msi_irqs(struct pci_dev *pdev)
static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
{
- irq_hw_number_t hwirq;
unsigned int virq;
struct msi_desc *entry;
struct msi_msg msg;
u64 addr;
- int ret;
+ int hwirq;
addr = find_ht_magic_addr(pdev);
msg.address_lo = addr & 0xFFFFFFFF;
msg.address_hi = addr >> 32;
list_for_each_entry(entry, &pdev->msi_list, list) {
- ret = mpic_msi_alloc_hwirqs(msi_mpic, 1);
- if (ret < 0) {
+ hwirq = msi_bitmap_alloc_hwirqs(&msi_mpic->msi_bitmap, 1);
+ if (hwirq < 0) {
pr_debug("u3msi: failed allocating hwirq\n");
- return ret;
+ return hwirq;
}
- hwirq = ret;
virq = irq_create_mapping(msi_mpic->irqhost, hwirq);
if (virq == NO_IRQ) {
- pr_debug("u3msi: failed mapping hwirq 0x%lx\n", hwirq);
- mpic_msi_free_hwirqs(msi_mpic, hwirq, 1);
+ pr_debug("u3msi: failed mapping hwirq 0x%x\n", hwirq);
+ msi_bitmap_free_hwirqs(&msi_mpic->msi_bitmap, hwirq, 1);
return -ENOSPC;
}
@@ -140,8 +140,8 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
set_irq_chip(virq, &mpic_u3msi_chip);
set_irq_type(virq, IRQ_TYPE_EDGE_RISING);
- pr_debug("u3msi: allocated virq 0x%x (hw 0x%lx) addr 0x%lx\n",
- virq, hwirq, addr);
+ pr_debug("u3msi: allocated virq 0x%x (hw 0x%x) addr 0x%lx\n",
+ virq, hwirq, (unsigned long)addr);
msg.data = hwirq;
write_msi_msg(virq, &msg);
--
1.5.5
^ permalink raw reply related
* [PATCH 3/4] powerpc: Convert the FSL MSI code to use msi_bitmap
From: Michael Ellerman @ 2008-08-05 23:10 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <b3997f77b701a0886df6eb3a5e4446d4dc538483.1217977742.git.michael@ellerman.id.au>
This is 90% straight forward, although we have to change a few
printk format strings as well because of the change in type of hwirq.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/sysdev/fsl_msi.c | 103 ++++++-----------------------------------
arch/powerpc/sysdev/fsl_msi.h | 5 +-
2 files changed, 17 insertions(+), 91 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index d49fa99..f25ce81 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -14,7 +14,6 @@
*/
#include <linux/irq.h>
#include <linux/bootmem.h>
-#include <linux/bitmap.h>
#include <linux/msi.h>
#include <linux/pci.h>
#include <linux/of_platform.h>
@@ -67,96 +66,22 @@ static struct irq_host_ops fsl_msi_host_ops = {
.map = fsl_msi_host_map,
};
-static irq_hw_number_t fsl_msi_alloc_hwirqs(struct fsl_msi *msi, int num)
-{
- unsigned long flags;
- int order = get_count_order(num);
- int offset;
-
- spin_lock_irqsave(&msi->bitmap_lock, flags);
-
- offset = bitmap_find_free_region(msi->fsl_msi_bitmap,
- NR_MSI_IRQS, order);
-
- spin_unlock_irqrestore(&msi->bitmap_lock, flags);
-
- pr_debug("%s: allocated 0x%x (2^%d) at offset 0x%x\n",
- __func__, num, order, offset);
-
- return offset;
-}
-
-static void fsl_msi_free_hwirqs(struct fsl_msi *msi, int offset, int num)
-{
- unsigned long flags;
- int order = get_count_order(num);
-
- pr_debug("%s: freeing 0x%x (2^%d) at offset 0x%x\n",
- __func__, num, order, offset);
-
- spin_lock_irqsave(&msi->bitmap_lock, flags);
- bitmap_release_region(msi->fsl_msi_bitmap, offset, order);
- spin_unlock_irqrestore(&msi->bitmap_lock, flags);
-}
-
-static int fsl_msi_free_dt_hwirqs(struct fsl_msi *msi)
-{
- int i;
- int len;
- const u32 *p;
-
- bitmap_allocate_region(msi->fsl_msi_bitmap, 0,
- get_count_order(NR_MSI_IRQS));
-
- p = of_get_property(msi->irqhost->of_node, "msi-available-ranges",
- &len);
-
- if (!p) {
- /* No msi-available-ranges property,
- * All the 256 MSI interrupts can be used
- */
- fsl_msi_free_hwirqs(msi, 0, 0x100);
- return 0;
- }
-
- if ((len % (2 * sizeof(u32))) != 0) {
- printk(KERN_WARNING "fsl_msi: Malformed msi-available-ranges "
- "property on %s\n", msi->irqhost->of_node->full_name);
- return -EINVAL;
- }
-
- /* Format is: (<u32 start> <u32 count>)+ */
- len /= 2 * sizeof(u32);
- for (i = 0; i < len; i++, p += 2)
- fsl_msi_free_hwirqs(msi, *p, *(p + 1));
-
- return 0;
-}
-
static int fsl_msi_init_allocator(struct fsl_msi *msi_data)
{
int rc;
- int size = BITS_TO_LONGS(NR_MSI_IRQS) * sizeof(u32);
- msi_data->fsl_msi_bitmap = kzalloc(size, GFP_KERNEL);
+ rc = msi_bitmap_alloc(&msi_data->bitmap, NR_MSI_IRQS,
+ msi_data->irqhost->of_node);
+ if (rc)
+ return rc;
- if (msi_data->fsl_msi_bitmap == NULL) {
- pr_debug("%s: ENOMEM allocating allocator bitmap!\n",
- __func__);
- return -ENOMEM;
+ rc = msi_bitmap_reserve_dt_hwirqs(&msi_data->bitmap);
+ if (rc < 0) {
+ msi_bitmap_free(&msi_data->bitmap);
+ return rc;
}
- rc = fsl_msi_free_dt_hwirqs(msi_data);
- if (rc)
- goto out_free;
-
return 0;
-out_free:
- kfree(msi_data->fsl_msi_bitmap);
-
- msi_data->fsl_msi_bitmap = NULL;
- return rc;
-
}
static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int type)
@@ -176,7 +101,8 @@ static void fsl_teardown_msi_irqs(struct pci_dev *pdev)
if (entry->irq == NO_IRQ)
continue;
set_irq_msi(entry->irq, NULL);
- fsl_msi_free_hwirqs(msi_data, virq_to_hw(entry->irq), 1);
+ msi_bitmap_free_hwirqs(&msi_data->bitmap,
+ virq_to_hw(entry->irq), 1);
irq_dispose_mapping(entry->irq);
}
@@ -198,15 +124,14 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int hwirq,
static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
{
- irq_hw_number_t hwirq;
- int rc;
+ int rc, hwirq;
unsigned int virq;
struct msi_desc *entry;
struct msi_msg msg;
struct fsl_msi *msi_data = fsl_msi;
list_for_each_entry(entry, &pdev->msi_list, list) {
- hwirq = fsl_msi_alloc_hwirqs(msi_data, 1);
+ hwirq = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
if (hwirq < 0) {
rc = hwirq;
pr_debug("%s: fail allocating msi interrupt\n",
@@ -217,9 +142,9 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
virq = irq_create_mapping(msi_data->irqhost, hwirq);
if (virq == NO_IRQ) {
- pr_debug("%s: fail mapping hwirq 0x%lx\n",
+ pr_debug("%s: fail mapping hwirq 0x%x\n",
__func__, hwirq);
- fsl_msi_free_hwirqs(msi_data, hwirq, 1);
+ msi_bitmap_free_hwirqs(&msi_data->bitmap, hwirq, 1);
rc = -ENOSPC;
goto out_free;
}
diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h
index 6574550..331c7e7 100644
--- a/arch/powerpc/sysdev/fsl_msi.h
+++ b/arch/powerpc/sysdev/fsl_msi.h
@@ -13,6 +13,8 @@
#ifndef _POWERPC_SYSDEV_FSL_MSI_H
#define _POWERPC_SYSDEV_FSL_MSI_H
+#include <asm/msi_bitmap.h>
+
#define NR_MSI_REG 8
#define IRQS_PER_MSI_REG 32
#define NR_MSI_IRQS (NR_MSI_REG * IRQS_PER_MSI_REG)
@@ -31,8 +33,7 @@ struct fsl_msi {
void __iomem *msi_regs;
u32 feature;
- unsigned long *fsl_msi_bitmap;
- spinlock_t bitmap_lock;
+ struct msi_bitmap bitmap;
};
#endif /* _POWERPC_SYSDEV_FSL_MSI_H */
--
1.5.5
^ permalink raw reply related
* [PATCH 2/4] powerpc: Split-out common MSI bitmap logic into msi_bitmap.c
From: Michael Ellerman @ 2008-08-05 23:10 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <b3997f77b701a0886df6eb3a5e4446d4dc538483.1217977742.git.michael@ellerman.id.au>
There are now two almost identical implementations of an MSI bitmap
allocator, one in mpic_msi.c and the other in fsl_msi.c.
Merge them together and put the result in msi_bitmap.c. Some of the
MPIC bits will remain to provide a nicer interface for the MPIC users.
In the process we fix two buglets. The first is that the allocation
routines, now msi_bitmap_alloc_hwirqs(), returned an unsigned result,
even though they use -1 to indicate allocation failure. Although all
the callers were checking correctly, it is much better for the routine
to just return an int. At least until someone wants > ~2 billion MSIs.
The second buglet is that the device tree reservation logic only
allowed power-of-two reservations. AFAICT that didn't effect any existing
code but it's nicer if we can reserve arbitrary irqs from MSI use.
We also add some selftests, which exposed the two buglets and now test
for them, as well as some basic sanity tests. The tests are only built
when CONFIG_DEBUG_KERNEL=y.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/Kconfig.debug | 5 +
arch/powerpc/include/asm/msi_bitmap.h | 35 +++++
arch/powerpc/sysdev/Kconfig | 6 +
arch/powerpc/sysdev/Makefile | 1 +
arch/powerpc/sysdev/msi_bitmap.c | 247 +++++++++++++++++++++++++++++++++
5 files changed, 294 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 4ebc52a..15eb278 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -51,6 +51,11 @@ config FTR_FIXUP_SELFTEST
depends on DEBUG_KERNEL
default n
+config MSI_BITMAP_SELFTEST
+ bool "Run self-tests of the MSI bitmap code."
+ depends on DEBUG_KERNEL
+ default n
+
config XMON
bool "Include xmon kernel debugger"
depends on DEBUG_KERNEL
diff --git a/arch/powerpc/include/asm/msi_bitmap.h b/arch/powerpc/include/asm/msi_bitmap.h
new file mode 100644
index 0000000..97ac3f4
--- /dev/null
+++ b/arch/powerpc/include/asm/msi_bitmap.h
@@ -0,0 +1,35 @@
+#ifndef _POWERPC_SYSDEV_MSI_BITMAP_H
+#define _POWERPC_SYSDEV_MSI_BITMAP_H
+
+/*
+ * Copyright 2008, Michael Ellerman, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2 of the
+ * License.
+ *
+ */
+
+#include <linux/of.h>
+#include <asm/irq.h>
+
+struct msi_bitmap {
+ struct device_node *of_node;
+ unsigned long *bitmap;
+ spinlock_t lock;
+ unsigned int irq_count;
+};
+
+int msi_bitmap_alloc_hwirqs(struct msi_bitmap *bmp, int num);
+void msi_bitmap_free_hwirqs(struct msi_bitmap *bmp, unsigned int offset,
+ unsigned int num);
+void msi_bitmap_reserve_hwirq(struct msi_bitmap *bmp, unsigned int hwirq);
+
+int msi_bitmap_reserve_dt_hwirqs(struct msi_bitmap *bmp);
+
+int msi_bitmap_alloc(struct msi_bitmap *bmp, unsigned int irq_count,
+ struct device_node *of_node);
+void msi_bitmap_free(struct msi_bitmap *bmp);
+
+#endif /* _POWERPC_SYSDEV_MSI_BITMAP_H */
diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig
index 72fb35b..3965828 100644
--- a/arch/powerpc/sysdev/Kconfig
+++ b/arch/powerpc/sysdev/Kconfig
@@ -6,3 +6,9 @@ config PPC4xx_PCI_EXPRESS
bool
depends on PCI && 4xx
default n
+
+config PPC_MSI_BITMAP
+ bool
+ depends on PCI_MSI
+ default y if MPIC
+ default y if FSL_PCI
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index a90054b..b6c269e 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -5,6 +5,7 @@ endif
mpic-msi-obj-$(CONFIG_PCI_MSI) += mpic_msi.o mpic_u3msi.o mpic_pasemi_msi.o
obj-$(CONFIG_MPIC) += mpic.o $(mpic-msi-obj-y)
fsl-msi-obj-$(CONFIG_PCI_MSI) += fsl_msi.o
+obj-$(CONFIG_PPC_MSI_BITMAP) += msi_bitmap.o
obj-$(CONFIG_PPC_MPC106) += grackle.o
obj-$(CONFIG_PPC_DCR_NATIVE) += dcr-low.o
diff --git a/arch/powerpc/sysdev/msi_bitmap.c b/arch/powerpc/sysdev/msi_bitmap.c
new file mode 100644
index 0000000..f84217b
--- /dev/null
+++ b/arch/powerpc/sysdev/msi_bitmap.c
@@ -0,0 +1,247 @@
+/*
+ * Copyright 2006-2008, Michael Ellerman, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2 of the
+ * License.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/bitmap.h>
+#include <asm/msi_bitmap.h>
+
+int msi_bitmap_alloc_hwirqs(struct msi_bitmap *bmp, int num)
+{
+ unsigned long flags;
+ int offset, order = get_count_order(num);
+
+ spin_lock_irqsave(&bmp->lock, flags);
+ /*
+ * This is fast, but stricter than we need. We might want to add
+ * a fallback routine which does a linear search with no alignment.
+ */
+ offset = bitmap_find_free_region(bmp->bitmap, bmp->irq_count, order);
+ spin_unlock_irqrestore(&bmp->lock, flags);
+
+ pr_debug("msi_bitmap: allocated 0x%x (2^%d) at offset 0x%x\n",
+ num, order, offset);
+
+ return offset;
+}
+
+void msi_bitmap_free_hwirqs(struct msi_bitmap *bmp, unsigned int offset,
+ unsigned int num)
+{
+ unsigned long flags;
+ int order = get_count_order(num);
+
+ pr_debug("msi_bitmap: freeing 0x%x (2^%d) at offset 0x%x\n",
+ num, order, offset);
+
+ spin_lock_irqsave(&bmp->lock, flags);
+ bitmap_release_region(bmp->bitmap, offset, order);
+ spin_unlock_irqrestore(&bmp->lock, flags);
+}
+
+void msi_bitmap_reserve_hwirq(struct msi_bitmap *bmp, unsigned int hwirq)
+{
+ unsigned long flags;
+
+ pr_debug("msi_bitmap: reserving hwirq 0x%x\n", hwirq);
+
+ spin_lock_irqsave(&bmp->lock, flags);
+ bitmap_allocate_region(bmp->bitmap, hwirq, 0);
+ spin_unlock_irqrestore(&bmp->lock, flags);
+}
+
+/**
+ * msi_bitmap_reserve_dt_hwirqs - Reserve irqs specified in the device tree.
+ * @bmp: pointer to the MSI bitmap.
+ *
+ * Looks in the device tree to see if there is a property specifying which
+ * irqs can be used for MSI. If found those irqs reserved in the device tree
+ * are reserved in the bitmap.
+ *
+ * Returns 0 for success, < 0 if there was an error, and > 0 if no property
+ * was found in the device tree.
+ **/
+int msi_bitmap_reserve_dt_hwirqs(struct msi_bitmap *bmp)
+{
+ int i, j, len;
+ const u32 *p;
+
+ if (!bmp->of_node)
+ return 1;
+
+ p = of_get_property(bmp->of_node, "msi-available-ranges", &len);
+ if (!p) {
+ pr_debug("msi_bitmap: no msi-available-ranges property " \
+ "found on %s\n", bmp->of_node->full_name);
+ return 1;
+ }
+
+ if (len % (2 * sizeof(u32)) != 0) {
+ printk(KERN_WARNING "msi_bitmap: Malformed msi-available-ranges"
+ " property on %s\n", bmp->of_node->full_name);
+ return -EINVAL;
+ }
+
+ bitmap_allocate_region(bmp->bitmap, 0, get_count_order(bmp->irq_count));
+
+ spin_lock(&bmp->lock);
+
+ /* Format is: (<u32 start> <u32 count>)+ */
+ len /= 2 * sizeof(u32);
+ for (i = 0; i < len; i++, p += 2) {
+ for (j = 0; j < *(p + 1); j++)
+ bitmap_release_region(bmp->bitmap, *p + j, 0);
+ }
+
+ spin_unlock(&bmp->lock);
+
+ return 0;
+}
+
+int msi_bitmap_alloc(struct msi_bitmap *bmp, unsigned int irq_count,
+ struct device_node *of_node)
+{
+ int size;
+
+ if (!irq_count)
+ return -EINVAL;
+
+ size = BITS_TO_LONGS(irq_count) * sizeof(long);
+ pr_debug("msi_bitmap: allocator bitmap size is 0x%x bytes\n", size);
+
+ bmp->bitmap = zalloc_maybe_bootmem(size, GFP_KERNEL);
+ if (!bmp->bitmap) {
+ pr_debug("msi_bitmap: ENOMEM allocating allocator bitmap!\n");
+ return -ENOMEM;
+ }
+
+ /* We zalloc'ed the bitmap, so all irqs are free by default */
+ spin_lock_init(&bmp->lock);
+ bmp->of_node = of_node_get(of_node);
+ bmp->irq_count = irq_count;
+
+ return 0;
+}
+
+void msi_bitmap_free(struct msi_bitmap *bmp)
+{
+ /* we can't free the bitmap we don't know if it's bootmem etc. */
+ of_node_put(bmp->of_node);
+ bmp->bitmap = NULL;
+}
+
+#ifdef CONFIG_MSI_BITMAP_SELFTEST
+
+#define check(x) \
+ if (!(x)) printk("msi_bitmap: test failed at line %d\n", __LINE__);
+
+void test_basics(void)
+{
+ struct msi_bitmap bmp;
+ int i, size = 512;
+
+ /* Can't allocate a bitmap of 0 irqs */
+ check(msi_bitmap_alloc(&bmp, 0, NULL) != 0);
+
+ /* of_node may be NULL */
+ check(0 == msi_bitmap_alloc(&bmp, size, NULL));
+
+ /* Should all be free by default */
+ check(0 == bitmap_find_free_region(bmp.bitmap, size,
+ get_count_order(size)));
+ bitmap_release_region(bmp.bitmap, 0, get_count_order(size));
+
+ /* With no node, there's no msi-available-ranges, so expect > 0 */
+ check(msi_bitmap_reserve_dt_hwirqs(&bmp) > 0);
+
+ /* Should all still be free */
+ check(0 == bitmap_find_free_region(bmp.bitmap, size,
+ get_count_order(size)));
+ bitmap_release_region(bmp.bitmap, 0, get_count_order(size));
+
+ /* Check we can fill it up and then no more */
+ for (i = 0; i < size; i++)
+ check(msi_bitmap_alloc_hwirqs(&bmp, 1) >= 0);
+
+ check(msi_bitmap_alloc_hwirqs(&bmp, 1) < 0);
+
+ /* Should all be allocated */
+ check(bitmap_find_free_region(bmp.bitmap, size, 0) < 0);
+
+ /* And if we free one we can then allocate another */
+ msi_bitmap_free_hwirqs(&bmp, size / 2, 1);
+ check(msi_bitmap_alloc_hwirqs(&bmp, 1) == size / 2);
+
+ msi_bitmap_free(&bmp);
+
+ /* Clients may check bitmap == NULL for "not-allocated" */
+ check(bmp.bitmap == NULL);
+
+ kfree(bmp.bitmap);
+}
+
+void test_of_node(void)
+{
+ u32 prop_data[] = { 10, 10, 25, 3, 40, 1, 100, 100, 200, 20 };
+ const char *expected_str = "0-9,20-24,28-39,41-99,220-255";
+ char *prop_name = "msi-available-ranges";
+ char *node_name = "/fakenode";
+ struct device_node of_node;
+ struct property prop;
+ struct msi_bitmap bmp;
+ int size = 256;
+ DECLARE_BITMAP(expected, size);
+
+ /* There should really be a struct device_node allocator */
+ memset(&of_node, 0, sizeof(of_node));
+ kref_init(&of_node.kref);
+ of_node.full_name = node_name;
+
+ check(0 == msi_bitmap_alloc(&bmp, size, &of_node));
+
+ /* No msi-available-ranges, so expect > 0 */
+ check(msi_bitmap_reserve_dt_hwirqs(&bmp) > 0);
+
+ /* Should all still be free */
+ check(0 == bitmap_find_free_region(bmp.bitmap, size,
+ get_count_order(size)));
+ bitmap_release_region(bmp.bitmap, 0, get_count_order(size));
+
+ /* Now create a fake msi-available-ranges property */
+
+ /* There should really .. oh whatever */
+ memset(&prop, 0, sizeof(prop));
+ prop.name = prop_name;
+ prop.value = &prop_data;
+ prop.length = sizeof(prop_data);
+
+ of_node.properties = ∝
+
+ /* msi-available-ranges, so expect == 0 */
+ check(msi_bitmap_reserve_dt_hwirqs(&bmp) == 0);
+
+ /* Check we got the expected result */
+ check(0 == bitmap_parselist(expected_str, expected, size));
+ check(bitmap_equal(expected, bmp.bitmap, size));
+
+ msi_bitmap_free(&bmp);
+ kfree(bmp.bitmap);
+}
+
+int msi_bitmap_selftest(void)
+{
+ printk(KERN_DEBUG "Running MSI bitmap self-tests ...\n");
+
+ test_basics();
+ test_of_node();
+
+ return 0;
+}
+late_initcall(msi_bitmap_selftest);
+#endif /* CONFIG_MSI_BITMAP_SELFTEST */
--
1.5.5
^ permalink raw reply related
* [PATCH 1/4] powerpc: fsl_msi doesn't need it's own of_node
From: Michael Ellerman @ 2008-08-05 23:10 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
The FSL MSI code keeps a pointer to the of_node from the device
it represents. However it also has an irq_host, which contains
a pointer to the of_node, so use that one instead.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/sysdev/fsl_msi.c | 12 +++++-------
arch/powerpc/sysdev/fsl_msi.h | 3 ---
2 files changed, 5 insertions(+), 10 deletions(-)
Hi Paul, I sent these a while ago, and they were acked by FSL folks and
Ojn - so I think Benh just forgot to merge them for 27.
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index 2c5187c..d49fa99 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -108,7 +108,8 @@ static int fsl_msi_free_dt_hwirqs(struct fsl_msi *msi)
bitmap_allocate_region(msi->fsl_msi_bitmap, 0,
get_count_order(NR_MSI_IRQS));
- p = of_get_property(msi->of_node, "msi-available-ranges", &len);
+ p = of_get_property(msi->irqhost->of_node, "msi-available-ranges",
+ &len);
if (!p) {
/* No msi-available-ranges property,
@@ -120,7 +121,7 @@ static int fsl_msi_free_dt_hwirqs(struct fsl_msi *msi)
if ((len % (2 * sizeof(u32))) != 0) {
printk(KERN_WARNING "fsl_msi: Malformed msi-available-ranges "
- "property on %s\n", msi->of_node->full_name);
+ "property on %s\n", msi->irqhost->of_node->full_name);
return -EINVAL;
}
@@ -317,14 +318,11 @@ static int __devinit fsl_of_msi_probe(struct of_device *dev,
goto error_out;
}
- msi->of_node = of_node_get(dev->node);
+ msi->irqhost = irq_alloc_host(dev->node, IRQ_HOST_MAP_LINEAR,
+ NR_MSI_IRQS, &fsl_msi_host_ops, 0);
- msi->irqhost = irq_alloc_host(of_node_get(dev->node),
- IRQ_HOST_MAP_LINEAR,
- NR_MSI_IRQS, &fsl_msi_host_ops, 0);
if (msi->irqhost == NULL) {
dev_err(&dev->dev, "No memory for MSI irqhost\n");
- of_node_put(dev->node);
err = -ENOMEM;
goto error_out;
}
diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h
index a653468..6574550 100644
--- a/arch/powerpc/sysdev/fsl_msi.h
+++ b/arch/powerpc/sysdev/fsl_msi.h
@@ -22,9 +22,6 @@
#define FSL_PIC_IP_IPIC 0x00000002
struct fsl_msi {
- /* Device node of the MSI interrupt*/
- struct device_node *of_node;
-
struct irq_host *irqhost;
unsigned long cascade_irq;
--
1.5.5
^ permalink raw reply related
* Re: to schedule() or not to schedule() ?
From: Michael Ellerman @ 2008-08-05 23:00 UTC (permalink / raw)
To: Kevin Diggs; +Cc: linuxppc-dev
In-Reply-To: <4898A96B.40502@hypersurf.com>
[-- Attachment #1: Type: text/plain, Size: 2210 bytes --]
On Tue, 2008-08-05 at 12:26 -0700, Kevin Diggs wrote:
> Chris Friesen wrote:
> > Kevin Diggs wrote:
> >> I have the following near the top of my cpufreq driver target
> >> routine:
> >>
> >> while(test_and_set_bit(cf750gxmCfgChangeBit,&cf750gxvStateBits)) {
> >> /*
> >> * Someone mucking with our cfg? (I hope it is ok to call
> >> * schedule() here! - truth is I have no idea what I am doing
> >> * ... my reasoning is I want to yeild the cpu so whoever is
> >> * mucking around can finish)
> >> */
> >> schedule();
> >> }
> >>
> >> This is to prevent bad things from happening if someone is trying to
> >> change a parameter for the driver via sysfs while the target routine
> >> is running. Fortunately, because I had a bug where this bit was not
> >> getting cleared on one of the paths through the target routine ... I
> >> now know it is not safe to call schedule (it got stuck in there -
> >> knocked out my adb keyboard! - (I think target is called from a timer
> >> that the governor sets up ... interrupt context?)).
> >
> >
> > Is the issue that someone may be in the middle of a multi-stage
> > procedure, and you've woken up partway through?
> >
> > If so, what about simply rescheduling the timer for some short time in
> > the future and aborting the current call?
> Chris,
>
> Thanks for taking the time to reply. The parameter in question modifies
> the frequency table. It is used several times in the target routine.
> I've addressed the issue by making a local copy of the frequency table
> upon entry to the target routine and use that while there. I don't care
> who wins the race.
How are you copying the table? Is it an atomic copy? Otherwise you could
just end up copying the table while it's being updated, and you get a
copy of the partially updated table.
Don't you just need a spinlock?
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: Kconfig debug help
From: John Linn @ 2008-08-05 22:50 UTC (permalink / raw)
To: jwboyer; +Cc: linuxppc-dev
In-Reply-To: <1217976528.2328.54.camel@localhost.localdomain>
Thanks Josh, I just came to the conclusion it was busted somehow in the
mainline after repeating it there.
-- John
> -----Original Message-----
> From: Josh Boyer [mailto:jwboyer@gmail.com] On Behalf Of Josh Boyer
> Sent: Tuesday, August 05, 2008 4:49 PM
> To: John Linn
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: Kconfig debug help
> =
> On Tue, 2008-08-05 at 15:37 -0600, John Linn wrote:
> > I'm trying to debug some Kconfig problems and am looking for a way
to
> > get more debug output.
> >
> >
> >
> > I have used KBUILD_VERBOSE=3D1 on the command line and it didn't help
> > much.
> >
> >
> >
> > I'm seeing an issue with creating a new defconfig file for a board.
I
> > get the .config created, then copy it to a defconfig, and then it
> > doesn't do anything when I run make with the defconfig file.
> =
> Yeah, I hit that myself this weekend. It's a bug in the Kconfig area.
> It's 1/2 fixed in the latest Linus and powerpc trees.
> =
> josh
> =
> =
This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.
^ permalink raw reply
* Re: Kconfig debug help
From: Josh Boyer @ 2008-08-05 22:48 UTC (permalink / raw)
To: John Linn; +Cc: linuxppc-dev
In-Reply-To: <20080805213759.96642158806D@mail101-wa4.bigfish.com>
On Tue, 2008-08-05 at 15:37 -0600, John Linn wrote:
> I’m trying to debug some Kconfig problems and am looking for a way to
> get more debug output.
>
>
>
> I have used KBUILD_VERBOSE=1 on the command line and it didn’t help
> much.
>
>
>
> I’m seeing an issue with creating a new defconfig file for a board. I
> get the .config created, then copy it to a defconfig, and then it
> doesn’t do anything when I run make with the defconfig file.
Yeah, I hit that myself this weekend. It's a bug in the Kconfig area.
It's 1/2 fixed in the latest Linus and powerpc trees.
josh
^ permalink raw reply
* Re: [PATCH add immr alias 2/4] powerpc: 5121: Add immr alias to MPC5121 ADS device tree.
From: John Rigby @ 2008-08-05 21:38 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, devicetree-discuss
In-Reply-To: <4898C434.4040408@freescale.com>
Scott Wood wrote:
> John Rigby wrote:
>> I would like to use mpc83xx_add_bridge for 5121. This is why I
>> moved it to fsl_pci.c. It currently uses get_immrbase and adds
>> 0x8300 and 0x8304 to it to pass to setup_indirect_pci as the
>> cfg_addr, and cfg_data addresses.
>>
>> I'm more than willing to change mpc83xx_add_bridge to not use
>> get_immrbase. One simple solution is to pass the cfg_addr and
>> cfg_data addresses in. If that seems ok then thats what I will do.
>
> We should really be putting those addresses in the "reg" property of
> the PCI node.
>
> -Scott
>
Yes, which is what fsl_add_bridge does it.
^ permalink raw reply
* Kconfig debug help
From: John Linn @ 2008-08-05 21:37 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 801 bytes --]
I'm trying to debug some Kconfig problems and am looking for a way to
get more debug output.
I have used KBUILD_VERBOSE=1 on the command line and it didn't help
much.
I'm seeing an issue with creating a new defconfig file for a board. I
get the .config created, then copy it to a defconfig, and then it
doesn't do anything when I run make with the defconfig file.
Thanks for any help,
John
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
[-- Attachment #2: Type: text/html, Size: 2808 bytes --]
^ permalink raw reply
* Re: [PATCH add immr alias 2/4] powerpc: 5121: Add immr alias to MPC5121 ADS device tree.
From: Scott Wood @ 2008-08-05 21:20 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev, devicetree-discuss
In-Reply-To: <4898C34C.5000305@freescale.com>
John Rigby wrote:
> I would like to use mpc83xx_add_bridge for 5121. This is why I
> moved it to fsl_pci.c. It currently uses get_immrbase and adds
> 0x8300 and 0x8304 to it to pass to setup_indirect_pci as the
> cfg_addr, and cfg_data addresses.
>
> I'm more than willing to change mpc83xx_add_bridge to not use
> get_immrbase. One simple solution is to pass the cfg_addr and
> cfg_data addresses in. If that seems ok then thats what I will do.
We should really be putting those addresses in the "reg" property of the
PCI node.
-Scott
^ permalink raw reply
* Re: [PATCH add immr alias 2/4] powerpc: 5121: Add immr alias to MPC5121 ADS device tree.
From: Scott Wood @ 2008-08-05 21:19 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, John Rigby
In-Reply-To: <fa686aa40808051412od699a57x59079bc59fb7150@mail.gmail.com>
Grant Likely wrote:
> But finding nodes that meet a criteria *is* what compatible is for and
> there is precedence for it. All u-boot platforms are finding the node
> by path right now, and so all of them need to be changed. Changing
> them to find by compatible that is set per-board or per-SoC makes
> complete sense to me.
It is ridiculous to have to duplicate code (or create a table, or
whatever) just so it can search for mpc8536-foo, mpc8544-foo,
mpc8548-foo, etc -- and in the case of the SoC, it's *not* fully
compatible, so we *can't* pick one as the "default" -- but it's
compatible for the purposes of the code in question.
I figured an alias would attract fewer flames than a compatible of
"fsl,immr" (though I'm fine with it -- it's specifying compatibility of
device tree binding, not of the hardware).
And no, they're not all finding it by path now -- there's a lot of use
of device_type "soc", which is what we're trying to avoid by introducing
this alias. The bootwrapper is also affected.
-Scott
^ permalink raw reply
* Re: [PATCH add immr alias 2/4] powerpc: 5121: Add immr alias to MPC5121 ADS device tree.
From: John Rigby @ 2008-08-05 21:17 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, devicetree-discuss
In-Reply-To: <fa686aa40808051405j7ea96957t9242e007c93d8e74@mail.gmail.com>
Uncle!
U-boot:
The 5121 currently fixes up the soc's bus-frequency node with a hard
coded path.
I'll leave it that way.
Kernel:
I would like to use mpc83xx_add_bridge for 5121. This is why I
moved it to fsl_pci.c. It currently uses get_immrbase and adds
0x8300 and 0x8304 to it to pass to setup_indirect_pci as the
cfg_addr, and cfg_data addresses.
I'm more than willing to change mpc83xx_add_bridge to not use
get_immrbase. One simple solution is to pass the cfg_addr and
cfg_data addresses in. If that seems ok then thats what I will do.
John
Grant Likely wrote:
> Oops, forgot to add devicetree-discuss to the cc: list
>
> g.
>
> On Tue, Aug 5, 2008 at 3:05 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
>
>> On Tue, Aug 5, 2008 at 2:13 PM, John Rigby <jrigby@freescale.com> wrote:
>>
>>> So get_immrbase can function without a device_type = "soc"
>>> property in the soc node.
>>>
>>> The "soc" node should really be named "immr"
>>> because it does not include the entire soc, however
>>> u-boot currently looks up this node by name for
>>> a clock fixup so leave it "soc" for now. We will change
>>> it later after 5121 u-boot uses the immr alias instead
>>> of the node name.
>>>
>> Is it not sufficient to search the tree for a node with the
>> <chip>-immr compatible value? I don't think this is the intended use
>> case of aliases.
>>
>> g.
>>
>> --
>> Grant Likely, B.Sc., P.Eng.
>> Secret Lab Technologies Ltd.
>>
>>
>
>
>
>
^ 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