* Re: [PATCH add immr alias 1/4] powerpc: Teach get_immrbase to use immr alias if it exists.
From: Grant Likely @ 2008-08-05 21:08 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, devicetree-discuss, John Rigby, Arnd Bergmann
In-Reply-To: <4898BCD3.5050703@freescale.com>
On Tue, Aug 5, 2008 at 2:49 PM, Scott Wood <scottwood@freescale.com> wrote:
> Arnd Bergmann wrote:
>>
>> On Tuesday 05 August 2008, John Rigby wrote:
>>>
>>> This will allow the eventual removal of device_type = "soc"
>>> properties in soc nodes.
>>
>> Stupid question, but why not remove immrbase instead?
>>
>> It seems that all users can be converted to use a reg
>> property of some actual device instead of making assumptions
>> about the register layout of the whole SOC.
>
> That wouldn't eliminate the need for the alias, though -- u-boot needs to
> find the node to fill in properties.
(already made this comment on one of the later patches, but it is more
relevant here...)
I don't think that using aliases is the best solution. I'd rather see
U-Boot search for the appropriate compatible value for the IMMR node.
g.
>
> -Scott
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ 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:08 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, John Rigby
In-Reply-To: <fa686aa40808051405j7268c216j8e5b526d6ad526c9@mail.gmail.com>
Grant Likely wrote:
> 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.
That get's really annoying in code that is meant to deal with multiple
chips. I don't think finding the network or serial node that
corresponds to u-boot's enumeration is what it's meant for, either, but
pretty much any alternative you can come up with has someone saying that
it's not what *that* was meant for, either.
-Scott
^ permalink raw reply
* Re: [PATCH add immr alias 2/4] powerpc: 5121: Add immr alias to MPC5121 ADS device tree.
From: Grant Likely @ 2008-08-05 21:12 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, John Rigby
In-Reply-To: <4898C15E.3000900@freescale.com>
On Tue, Aug 5, 2008 at 3:08 PM, Scott Wood <scottwood@freescale.com> wrote:
> Grant Likely wrote:
>>
>> 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.
>
> That get's really annoying in code that is meant to deal with multiple
> chips. I don't think finding the network or serial node that corresponds to
> u-boot's enumeration is what it's meant for, either, but pretty much any
> alternative you can come up with has someone saying that it's not what
> *that* was meant for, either.
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.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ 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
* 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: 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
* 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: 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
* 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: 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: 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
* [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
* [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 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 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
* 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
* 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
* 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: 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
* 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: [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: 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
* [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: 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
* 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
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