* Re: [PATCH 3/4] Simplify rtas_change_msi() error semantics
From: Michael Ellerman @ 2007-10-02 7:40 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
In-Reply-To: <1191306253.6310.106.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 1795 bytes --]
On Tue, 2007-10-02 at 16:24 +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2007-10-02 at 15:58 +1000, Michael Ellerman wrote:
> > > Looks allright, just a question tho... what do we do if it fails ?
> > Do we
> > > try to fallback to a lower number of MSIs ? Or what ? Dead device ?
> >
> > That's all up to the device driver. In theory the driver could try again
> > with a lower count - but that might require extra logic in the driver to
> > handle shared irq handlers etc. In practice I think the current drivers
> > will just fail.
>
> Question is badly phrased.. I meant something more like... what do we do
> if RTAS returns a lower count ?
>
> That is, we end up with that device with that lower count of MSIs
> enabled, we fail at the driver level, do we still somewhat keep track ?
> Drivers might assume that means it can use LSIs no ? which may not be
> the case... Shouldn't we try to switch back to LSI mode (or does the
> RTAS interface doesnt allow it ?)
OK I get you. So:
rtas_setup_msi_irqs() detects that it got fewer MSIs than it asked for
and returns an error.
The generic code (drivers/pci/msi.c) notices the error and calls
msi_free_irqs().
That calls back into rtas_teardown_msi_irqs() which disposes of the virq
mappings and calls rtas_disable_msi().
rtas_disable_msi() asks firmware to configure 0 MSIs on the device, that
hopefully succeeds. AFAIK configuring 0 MSIs is as close as we can get
to disabling MSI via RTAS.
Perhaps that should also (re)enable INTX?
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] cpm: rounding of brg clockdivider
From: Esben Haabendal @ 2007-10-02 7:24 UTC (permalink / raw)
To: Scott Wood, LinuxPPC-dev
Do rounding of brg clockdivider instead of truncate to get more precise baudrates
Something similar might be needed for cpm_fastbrg...
---
commit 52d631eb8f64cef794d6aa66494e253cf268894e
tree 956149a0eb5beb9afb280f4593615929eab7b779
parent 300070dd6b5e71af0c6fbecd32388905dbdd3ea5
author Esben Haabendal <eha@doredevelopment.dk> Wed, 19 Sep 2007 13:18:59 +0200
committer Esben Haabendal <esben@esben.doredevelopment.dk> Wed, 19 Sep 2007 13:18:59 +0200
arch/powerpc/sysdev/cpm2_common.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/sysdev/cpm2_common.c b/arch/powerpc/sysdev/cpm2_common.c
index 9058da2..a2c8157 100644
--- a/arch/powerpc/sysdev/cpm2_common.c
+++ b/arch/powerpc/sysdev/cpm2_common.c
@@ -102,7 +102,9 @@ cpm_setbrg(uint brg, uint rate)
brg -= 4;
}
bp += brg;
- out_be32(bp, (((BRG_UART_CLK / rate) - 1) << 1) | CPM_BRG_EN);
+ /* Set the BRG clock divider to get the best match to the requested
+ * baudrate (rounding required) */
+ out_be32(bp, ((((((BRG_UART_CLK*2)/rate)+1)/2)-1) << 1) | CPM_BRG_EN);
cpm2_unmap(bp);
}
!-------------------------------------------------------------flip-
--
Esben Haabendal
Embedded Software Consultant
Doré Development ApS
^ permalink raw reply related
* RE: [PATCH 6/9] fs_enet: Be an of_platform device when CONFIG_PPC_CPM_NEW_BINDING is set.
From: Esben Haabendal @ 2007-10-02 7:10 UTC (permalink / raw)
To: 'Scott Wood'; +Cc: netdev, linuxppc-dev
In-Reply-To: <20070920220121.GF28784@loki.buserror.net>
Hi Scott,
A minor error handling bug
> + const u32 *data = of_get_property(np, "phy-handle", &len);
> + if (!data || len != 4)
> + return -EINVAL;
> +
> + phynode = of_find_node_by_phandle(*data);
> + if (!phynode)
> + return -EINVAL;
> +
> + mdionode = of_get_parent(phynode);
> + if (!phynode)
if (!mdionode)
> + goto out_put_phy;
Best regards,
Esben
^ permalink raw reply
* Re: Powerbook shuts down hard when hot, patch found
From: Michel Dänzer @ 2007-10-02 6:43 UTC (permalink / raw)
To: Michael Buesch; +Cc: linuxppc-dev
In-Reply-To: <200710012258.18716.mb@bu3sch.de>
On Mon, 2007-10-01 at 22:58 +0200, Michael Buesch wrote:
> On Monday 01 October 2007 10:00:02 Michel Dänzer wrote:
> >
> > On Sun, 2007-09-30 at 12:16 +0200, Michael Buesch wrote:
> > >
> > > Ah, forgot to say.
> > > It does not crash immediately when the register is written. It takes about two seconds
> > > to crash. And when the machine is colder to begin with, it takes slightly
> > > longer to trigger. That _might_ support your overheating theory.
> > >
> > > Though, it does not trigger when it's up and running, no matter how hot you drive it.
> >
> > Maybe the thermal control module prevents it from overheating. Have you
> > tried unloading it or loading it ASAP on bootup to see if that makes any
> > difference either way?
>
> I'm not sure how that works. Can I unload the framebuffer module?
I'm talking about the thermal control module (therm_adt746x in my case),
not radeonfb.
--
Earthling Michel Dänzer | http://tungstengraphics.com
Libre software enthusiast | Debian, X and DRI developer
^ permalink raw reply
* Re: [PATCH v2 2/6] Sysace: Use the established platform bus api
From: Jens Axboe @ 2007-10-02 6:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Christoph Hellwig, paulus, linux-kernel, linuxppc-dev
In-Reply-To: <1191304710.6310.97.camel@pasglop>
On Tue, Oct 02 2007, Benjamin Herrenschmidt wrote:
>
> On Mon, 2007-10-01 at 13:59 +0200, Jens Axboe wrote:
> > On Sun, Sep 30 2007, Grant Likely wrote:
> > > On 9/30/07, Christoph Hellwig <hch@infradead.org> wrote:
> > > > On Sun, Sep 30, 2007 at 04:57:09PM -0600, Grant Likely wrote:
> > > > > + if ((rc = platform_driver_register(&ace_platform_driver)) != 0)
> > > > > + goto err_plat;
> > > >
> > > > rc = platform_driver_register(&ace_platform_driver);
> > > > if (rc)
> > > > goto err_plat;
> > > >
> > > > please.
> > >
> > > Okay, will do.
> > >
> > > >
> > > > > + err_plat:
> > > > > + unregister_blkdev(ace_major, "xsysace");
> > > > > + err_blk:
> > > >
> > > > labels should be indented zero or one space, but not more.
> > >
> > > scripts/Lindent does this. Originally, I *didn't* have my labels
> > > indented. :-) Does Lindent need to be fixed?
> >
> > Seems so, if it idents labels.
> >
> > Just send a fixup patch for that, I'll add your series to the block tree
> > for 2.6.24.
>
> It's actually better off living in the powerpc tree I think as it's
> really about adding support for a new powerpc platform and somewhat
> needs to sync with other things in there. Unless you really want the
> whole thing in your tree :-)
I already included it yesterday, it'll go up once 2.6.24 opens. Let me
know if you want me to rip it out, though.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 3/4] Simplify rtas_change_msi() error semantics
From: Benjamin Herrenschmidt @ 2007-10-02 6:24 UTC (permalink / raw)
To: michael; +Cc: linuxppc-dev
In-Reply-To: <1191304690.6593.8.camel@concordia>
On Tue, 2007-10-02 at 15:58 +1000, Michael Ellerman wrote:
> > Looks allright, just a question tho... what do we do if it fails ?
> Do we
> > try to fallback to a lower number of MSIs ? Or what ? Dead device ?
>
> That's all up to the device driver. In theory the driver could try again
> with a lower count - but that might require extra logic in the driver to
> handle shared irq handlers etc. In practice I think the current drivers
> will just fail.
Question is badly phrased.. I meant something more like... what do we do
if RTAS returns a lower count ?
That is, we end up with that device with that lower count of MSIs
enabled, we fail at the driver level, do we still somewhat keep track ?
Drivers might assume that means it can use LSIs no ? which may not be
the case... Shouldn't we try to switch back to LSI mode (or does the
RTAS interface doesnt allow it ?)
Ben.
^ permalink raw reply
* Re: [PATCH 6/7] Add dcr_map_reg() helper
From: Benjamin Herrenschmidt @ 2007-10-02 6:22 UTC (permalink / raw)
To: michael; +Cc: linuxppc-dev
In-Reply-To: <1191304300.6593.2.camel@concordia>
On Tue, 2007-10-02 at 15:51 +1000, Michael Ellerman wrote:
> On Tue, 2007-10-02 at 15:19 +1000, Benjamin Herrenschmidt wrote:
> > On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> > > Add a helper routine to map dcr's based on the "dcr-reg" property of
> > > a device node.
> > >
> > > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> >
> > Wouldn't it be more consistent to call it of_map_dcr ? Or maybe find an
> > even better name, but dcr_map_reg really sucks :-)
>
> That would give us dcr_map(), dcr_unmap() and of_map_dcr() - which
> doesn't strike me as more consistent.
>
> I don't particularly like dcr_map_reg(), but I think it's at least
> obvious that it's a variant of dcr_map() and that it has something to do
> with a "reg" .. maybe even a "dcr-reg" :)
dcr_map_node maybe ? dcr_map_device ? dcr_map_property ? hrm...
allright, keep it that way if you want but heh, at least see if you can
come up with something better before it gets merged :-)
Ben
^ permalink raw reply
* Re: [PATCH 3/7] Use dcr_host_t.base in ibm_emac_mal
From: Michael Ellerman @ 2007-10-02 6:06 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
In-Reply-To: <1191302025.6310.70.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]
On Tue, 2007-10-02 at 15:13 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> > This requires us to do a sort-of fake dcr_map(), so that base is set
> > properly. This will be fixed/removed when the device-tree-aware emac driver
> > is merged.
> >
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
>
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>
> (Have you actually tested btw ? :-)
Hmm, I wrote these in mid June, so to be honest I don't remember. I
don't think I boot tested it on anything, but I think I would have built
it.
I see now that the newemac driver is in Jeff's 24 tree, so it's probably
best to wait for Linus to merge that and then we can fix up it as well
in the one series.
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: nap/dfs on 7448
From: Benjamin Herrenschmidt @ 2007-10-02 6:01 UTC (permalink / raw)
To: Leisner, Martin; +Cc: linuxppc-dev
In-Reply-To: <556445368AFA1C438794ABDA8901891C0738E1E8@USA0300MS03.na.xerox.net>
On Mon, 2007-10-01 at 17:32 -0400, Leisner, Martin wrote:
> I asked this on linuxppc-embedded a week ago (I didn't even know this
> list existed until last week -- another reason to get rid of
> linuxpcc-embedded).
>
> Has anyone gotten NAP/DFS to reliably work on a 7448?
>
> I'm seeing strange problems with peripherals...(using a ram disk
> works fine).
Could it be that your host bridge isn't properly waking up the CPU to
DOZE state for snooping DMA ? (It might require some delays on QACK in
some cases, I know Apple had workarounds in those areas, maybe something
along those lines need to be configured in the chipset).
Ben.
^ permalink raw reply
* Re: [PATCH v2 2/6] Sysace: Use the established platform bus api
From: Benjamin Herrenschmidt @ 2007-10-02 5:58 UTC (permalink / raw)
To: Jens Axboe; +Cc: Christoph Hellwig, paulus, linux-kernel, linuxppc-dev
In-Reply-To: <20071001115954.GD5303@kernel.dk>
On Mon, 2007-10-01 at 13:59 +0200, Jens Axboe wrote:
> On Sun, Sep 30 2007, Grant Likely wrote:
> > On 9/30/07, Christoph Hellwig <hch@infradead.org> wrote:
> > > On Sun, Sep 30, 2007 at 04:57:09PM -0600, Grant Likely wrote:
> > > > + if ((rc = platform_driver_register(&ace_platform_driver)) != 0)
> > > > + goto err_plat;
> > >
> > > rc = platform_driver_register(&ace_platform_driver);
> > > if (rc)
> > > goto err_plat;
> > >
> > > please.
> >
> > Okay, will do.
> >
> > >
> > > > + err_plat:
> > > > + unregister_blkdev(ace_major, "xsysace");
> > > > + err_blk:
> > >
> > > labels should be indented zero or one space, but not more.
> >
> > scripts/Lindent does this. Originally, I *didn't* have my labels
> > indented. :-) Does Lindent need to be fixed?
>
> Seems so, if it idents labels.
>
> Just send a fixup patch for that, I'll add your series to the block tree
> for 2.6.24.
It's actually better off living in the powerpc tree I think as it's
really about adding support for a new powerpc platform and somewhat
needs to sync with other things in there. Unless you really want the
whole thing in your tree :-)
Cheers
Ben.
^ permalink raw reply
* Re: [PATCH 3/4] Simplify rtas_change_msi() error semantics
From: Michael Ellerman @ 2007-10-02 5:58 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
In-Reply-To: <1191302603.6310.88.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]
On Tue, 2007-10-02 at 15:23 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> > Currently rtas_change_msi() returns either the error code from RTAS, or if
> > the RTAS call succeeded the number of irqs that were configured by RTAS.
> > This makes checking the return value more complicated than it needs to be.
> >
> > Instead, have rtas_change_msi() check that the number of irqs configured by
> > RTAS is equal to what we requested - and return an error otherwise. This makes
> > the return semantics match the usual 0 for success, something else for error.
> >
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
>
> Looks allright, just a question tho... what do we do if it fails ? Do we
> try to fallback to a lower number of MSIs ? Or what ? Dead device ?
That's all up to the device driver. In theory the driver could try again
with a lower count - but that might require extra logic in the driver to
handle shared irq handlers etc. In practice I think the current drivers
will just fail.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH v2 5/6] Sysace: Move IRQ handler registration to occur after FSM is initialized
From: Benjamin Herrenschmidt @ 2007-10-02 5:55 UTC (permalink / raw)
To: Grant Likely; +Cc: axboe, linuxppc-dev, paulus, linux-kernel
In-Reply-To: <20070930225726.2476.44211.stgit@trillian.cg.shawcable.net>
On Sun, 2007-09-30 at 16:57 -0600, Grant Likely wrote:
> val |= ACE_CTRL_DATABUFRDYIRQ | ACE_CTRL_ERRORIRQ;
> ace_out(ace, ACE_CTRL, val);
>
> + /* Now we can hook up the irq handler */
> + if (ace->irq != NO_IRQ) {
> + rc = request_irq(ace->irq, ace_interrupt, 0,
> "systemace", ace);
> + if (rc) {
> + /* Failure - fall back to polled mode */
> + dev_err(ace->dev, "request_irq failed\n");
> + ace->irq = NO_IRQ;
> + }
> + }
> +
I don't know the HW but from the above, it looks like you enable
interrupt emission on the HW before you register the handler, which is
wrong. You should make sure on the contrary that IRQs on the HW are
disabled until after you have registered a handler.
Only really a problem if you have shared interrupts but still...
Ben.
^ permalink raw reply
* Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding
From: Benjamin Herrenschmidt @ 2007-10-02 5:53 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <20070930224208.1871.2913.stgit@trillian.cg.shawcable.net>
On Sun, 2007-09-30 at 16:42 -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
>
> Add of_platform bus binding so this driver can be used with arch/powerpc
Another option is to have a "constructor" in the platform code that
generates the platform device from the DT. It might even become the
preferred way one of these days, I'm not too sure at this stage. Anyway,
do what you prefer.
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> drivers/serial/uartlite.c | 101 +++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 93 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
> index ed13b9f..8752fac 100644
> --- a/drivers/serial/uartlite.c
> +++ b/drivers/serial/uartlite.c
> @@ -1,7 +1,8 @@
> /*
> * uartlite.c: Serial driver for Xilinx uartlite serial controller
> *
> - * Peter Korsgaard <jacmet@sunsite.dk>
> + * Copyright (C) 2006 Peter Korsgaard <jacmet@sunsite.dk>
> + * Copyright (C) 2007 Secret Lab Technologies Ltd.
> *
> * This file is licensed under the terms of the GNU General Public License
> * version 2. This program is licensed "as is" without any warranty of any
> @@ -17,6 +18,10 @@
> #include <linux/delay.h>
> #include <linux/interrupt.h>
> #include <asm/io.h>
> +#if defined(CONFIG_OF)
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#endif
>
> #define ULITE_NAME "ttyUL"
> #define ULITE_MAJOR 204
> @@ -382,8 +387,10 @@ static int __init ulite_console_setup(struct console *co, char *options)
> port = &ulite_ports[co->index];
>
> /* not initialized yet? */
> - if (!port->membase)
> + if (!port->membase) {
> + pr_debug("console on ttyUL%i not initialized\n", co->index);
> return -ENODEV;
> + }
>
> if (options)
> uart_parse_options(options, &baud, &parity, &bits, &flow);
> @@ -542,6 +549,72 @@ static struct platform_driver ulite_platform_driver = {
> };
>
> /* ---------------------------------------------------------------------
> + * OF bus bindings
> + */
> +#if defined(CONFIG_OF)
> +static int __devinit
> +ulite_of_probe(struct of_device *op, const struct of_device_id *match)
> +{
> + struct resource res;
> + const unsigned int *id;
> + int irq, rc;
> +
> + dev_dbg(&op->dev, "%s(%p, %p)\n", __FUNCTION__, op, match);
> +
> + rc = of_address_to_resource(op->node, 0, &res);
> + if (rc) {
> + dev_err(&op->dev, "invalide address\n");
> + return rc;
> + }
> +
> + irq = irq_of_parse_and_map(op->node, 0);
> +
> + id = of_get_property(op->node, "port-number", NULL);
> +
> + return ulite_assign(&op->dev, id ? *id : -1, res.start, irq);
> +}
> +
> +static int __devexit ulite_of_remove(struct of_device *op)
> +{
> + return ulite_release(&op->dev);
> +}
> +
> +/* Match table for of_platform binding */
> +static struct of_device_id __devinit ulite_of_match[] = {
> + { .type = "serial", .compatible = "xilinx,uartlite", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ulite_of_match);
> +
> +static struct of_platform_driver ulite_of_driver = {
> + .owner = THIS_MODULE,
> + .name = "uartlite",
> + .match_table = ulite_of_match,
> + .probe = ulite_of_probe,
> + .remove = __devexit_p(ulite_of_remove),
> + .driver = {
> + .name = "uartlite",
> + },
> +};
> +
> +/* Registration helpers to keep the number of #ifdefs to a minimum */
> +static inline int __init ulite_of_register(void)
> +{
> + pr_debug("uartlite: calling of_register_platform_driver()\n");
> + return of_register_platform_driver(&ulite_of_driver);
> +}
> +
> +static inline void __exit ulite_of_unregister(void)
> +{
> + of_unregister_platform_driver(&ulite_of_driver);
> +}
> +#else /* CONFIG_OF */
> +/* CONFIG_OF not enabled; do nothing helpers */
> +static inline int __init ulite_of_register(void) { return 0; }
> +static inline void __exit ulite_of_unregister(void) { }
> +#endif /* CONFIG_OF */
> +
> +/* ---------------------------------------------------------------------
> * Module setup/teardown
> */
>
> @@ -549,20 +622,32 @@ int __init ulite_init(void)
> {
> int ret;
>
> - ret = uart_register_driver(&ulite_uart_driver);
> - if (ret)
> - return ret;
> + pr_debug("uartlite: calling uart_register_driver()\n");
> + if ((ret = uart_register_driver(&ulite_uart_driver)) != 0)
> + goto err_uart;
>
> - ret = platform_driver_register(&ulite_platform_driver);
> - if (ret)
> - uart_unregister_driver(&ulite_uart_driver);
> + if ((ret = ulite_of_register()) != 0)
> + goto err_of;
>
> + pr_debug("uartlite: calling platform_driver_register()\n");
> + if ((ret = platform_driver_register(&ulite_platform_driver)) != 0)
> + goto err_plat;
> +
> + return 0;
> +
> +err_plat:
> + ulite_of_unregister();
> +err_of:
> + uart_unregister_driver(&ulite_uart_driver);
> +err_uart:
> + printk(KERN_ERR "registering uartlite driver failed: err=%i", ret);
> return ret;
> }
>
> void __exit ulite_exit(void)
> {
> platform_driver_unregister(&ulite_platform_driver);
> + ulite_of_unregister();
> uart_unregister_driver(&ulite_uart_driver);
> }
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH 6/7] Add dcr_map_reg() helper
From: Michael Ellerman @ 2007-10-02 5:51 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
In-Reply-To: <1191302382.6310.80.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 980 bytes --]
On Tue, 2007-10-02 at 15:19 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> > Add a helper routine to map dcr's based on the "dcr-reg" property of
> > a device node.
> >
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
>
> Wouldn't it be more consistent to call it of_map_dcr ? Or maybe find an
> even better name, but dcr_map_reg really sucks :-)
That would give us dcr_map(), dcr_unmap() and of_map_dcr() - which
doesn't strike me as more consistent.
I don't particularly like dcr_map_reg(), but I think it's at least
obvious that it's a variant of dcr_map() and that it has something to do
with a "reg" .. maybe even a "dcr-reg" :)
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 v2] powerpc: Implement logging of unhandled signals
From: Olof Johansson @ 2007-10-02 5:35 UTC (permalink / raw)
To: paulus; +Cc: linuxppc-dev
In-Reply-To: <20071001203242.GA14091@lixom.net>
Implement show_unhandled_signals sysctl + support to print when a process
is killed due to unhandled signals just as i386 and x86_64 does.
Signed-off-by: Olof Johansson <olof@lixom.net>
---
Fixed the const char definitions, plus the two warnings that the change
brought (printing a ptr as %lx without cast).
Index: 2.6.23/arch/powerpc/kernel/traps.c
===================================================================
--- 2.6.23.orig/arch/powerpc/kernel/traps.c
+++ 2.6.23/arch/powerpc/kernel/traps.c
@@ -172,11 +172,21 @@ int die(const char *str, struct pt_regs
void _exception(int signr, struct pt_regs *regs, int code, unsigned long addr)
{
siginfo_t info;
+ const char fmt32[] = KERN_INFO "%s[%d]: unhandled signal %d " \
+ "at %08lx nip %08lx lr %08lx code %x\n";
+ const char fmt64[] = KERN_INFO "%s[%d]: unhandled signal %d " \
+ "at %016lx nip %016lx lr %016lx code %x\n";
if (!user_mode(regs)) {
if (die("Exception in kernel mode", regs, signr))
return;
- }
+ } else if (show_unhandled_signals &&
+ unhandled_signal(current, signr) &&
+ printk_ratelimit()) {
+ printk(regs->msr & MSR_SF ? fmt64 : fmt32,
+ current->comm, current->pid, signr,
+ addr, regs->nip, regs->link, code);
+ }
memset(&info, 0, sizeof(info));
info.si_signo = signr;
Index: 2.6.23/arch/powerpc/kernel/signal_64.c
===================================================================
--- 2.6.23.orig/arch/powerpc/kernel/signal_64.c
+++ 2.6.23/arch/powerpc/kernel/signal_64.c
@@ -64,6 +64,11 @@ struct rt_sigframe {
char abigap[288];
} __attribute__ ((aligned (16)));
+static const char fmt32[] = KERN_INFO \
+ "%s[%d]: bad frame in %s: %08lx nip %08lx lr %08lx\n";
+static const char fmt64[] = KERN_INFO \
+ "%s[%d]: bad frame in %s: %016lx nip %016lx lr %016lx\n";
+
/*
* Set up the sigcontext for the signal frame.
*/
@@ -315,6 +320,11 @@ badframe:
printk("badframe in sys_rt_sigreturn, regs=%p uc=%p &uc->uc_mcontext=%p\n",
regs, uc, &uc->uc_mcontext);
#endif
+ if (show_unhandled_signals && printk_ratelimit())
+ printk(regs->msr & MSR_SF ? fmt64 : fmt32,
+ current->comm, current->pid, "rt_sigreturn",
+ (long)uc, regs->nip, regs->link);
+
force_sig(SIGSEGV, current);
return 0;
}
@@ -398,6 +408,11 @@ badframe:
printk("badframe in setup_rt_frame, regs=%p frame=%p newsp=%lx\n",
regs, frame, newsp);
#endif
+ if (show_unhandled_signals && printk_ratelimit())
+ printk(regs->msr & MSR_SF ? fmt64 : fmt32,
+ current->comm, current->pid, "setup_rt_frame",
+ (long)frame, regs->nip, regs->link);
+
force_sigsegv(signr, current);
return 0;
}
Index: 2.6.23/kernel/sysctl.c
===================================================================
--- 2.6.23.orig/kernel/sysctl.c
+++ 2.6.23/kernel/sysctl.c
@@ -1221,7 +1221,7 @@ static ctl_table fs_table[] = {
};
static ctl_table debug_table[] = {
-#ifdef CONFIG_X86
+#if defined(CONFIG_X86) || defined(CONFIG_PPC)
{
.ctl_name = CTL_UNNUMBERED,
.procname = "exception-trace",
Index: 2.6.23/arch/powerpc/kernel/signal_32.c
===================================================================
--- 2.6.23.orig/arch/powerpc/kernel/signal_32.c
+++ 2.6.23/arch/powerpc/kernel/signal_32.c
@@ -705,11 +705,13 @@ int handle_rt_signal32(unsigned long sig
{
struct rt_sigframe __user *rt_sf;
struct mcontext __user *frame;
+ void __user *addr;
unsigned long newsp = 0;
/* Set up Signal Frame */
/* Put a Real Time Context onto stack */
rt_sf = get_sigframe(ka, regs, sizeof(*rt_sf));
+ addr = rt_sf;
if (unlikely(rt_sf == NULL))
goto badframe;
@@ -728,6 +730,7 @@ int handle_rt_signal32(unsigned long sig
/* Save user registers on the stack */
frame = &rt_sf->uc.uc_mcontext;
+ addr = frame;
if (vdso32_rt_sigtramp && current->mm->context.vdso_base) {
if (save_user_regs(regs, frame, 0))
goto badframe;
@@ -742,6 +745,7 @@ int handle_rt_signal32(unsigned long sig
/* create a stack frame for the caller of the handler */
newsp = ((unsigned long)rt_sf) - (__SIGNAL_FRAMESIZE + 16);
+ addr = (void __user *)regs->gpr[1];
if (put_user(regs->gpr[1], (u32 __user *)newsp))
goto badframe;
@@ -762,6 +766,12 @@ badframe:
printk("badframe in handle_rt_signal, regs=%p frame=%p newsp=%lx\n",
regs, frame, newsp);
#endif
+ if (show_unhandled_signals && printk_ratelimit())
+ printk(KERN_INFO "%s[%d]: bad frame in handle_rt_signal32: "
+ "%p nip %08lx lr %08lx\n",
+ current->comm, current->pid,
+ addr, regs->nip, regs->link);
+
force_sigsegv(sig, current);
return 0;
}
@@ -886,6 +896,12 @@ long sys_rt_sigreturn(int r3, int r4, in
return 0;
bad:
+ if (show_unhandled_signals && printk_ratelimit())
+ printk(KERN_INFO "%s[%d]: bad frame in sys_rt_sigreturn: "
+ "%p nip %08lx lr %08lx\n",
+ current->comm, current->pid,
+ rt_sf, regs->nip, regs->link);
+
force_sig(SIGSEGV, current);
return 0;
}
@@ -967,6 +983,13 @@ int sys_debug_setcontext(struct ucontext
* We kill the task with a SIGSEGV in this situation.
*/
if (do_setcontext(ctx, regs, 1)) {
+ if (show_unhandled_signals && printk_ratelimit())
+ printk(KERN_INFO "%s[%d]: bad frame in "
+ "sys_debug_setcontext: %p nip %08lx "
+ "lr %08lx\n",
+ current->comm, current->pid,
+ ctx, regs->nip, regs->link);
+
force_sig(SIGSEGV, current);
goto out;
}
@@ -1048,6 +1071,12 @@ badframe:
printk("badframe in handle_signal, regs=%p frame=%p newsp=%lx\n",
regs, frame, newsp);
#endif
+ if (show_unhandled_signals && printk_ratelimit())
+ printk(KERN_INFO "%s[%d]: bad frame in handle_signal32: "
+ "%p nip %08lx lr %08lx\n",
+ current->comm, current->pid,
+ frame, regs->nip, regs->link);
+
force_sigsegv(sig, current);
return 0;
}
@@ -1061,12 +1090,14 @@ long sys_sigreturn(int r3, int r4, int r
struct sigcontext __user *sc;
struct sigcontext sigctx;
struct mcontext __user *sr;
+ void __user *addr;
sigset_t set;
/* Always make any pending restarted system calls return -EINTR */
current_thread_info()->restart_block.fn = do_no_restart_syscall;
sc = (struct sigcontext __user *)(regs->gpr[1] + __SIGNAL_FRAMESIZE);
+ addr = sc;
if (copy_from_user(&sigctx, sc, sizeof(sigctx)))
goto badframe;
@@ -1083,6 +1114,7 @@ long sys_sigreturn(int r3, int r4, int r
restore_sigmask(&set);
sr = (struct mcontext __user *)from_user_ptr(sigctx.regs);
+ addr = sr;
if (!access_ok(VERIFY_READ, sr, sizeof(*sr))
|| restore_user_regs(regs, sr, 1))
goto badframe;
@@ -1091,6 +1123,12 @@ long sys_sigreturn(int r3, int r4, int r
return 0;
badframe:
+ if (show_unhandled_signals && printk_ratelimit())
+ printk(KERN_INFO "%s[%d]: bad frame in sys_sigreturn: "
+ "%p nip %08lx lr %08lx\n",
+ current->comm, current->pid,
+ addr, regs->nip, regs->link);
+
force_sig(SIGSEGV, current);
return 0;
}
Index: 2.6.23/arch/powerpc/kernel/signal.c
===================================================================
--- 2.6.23.orig/arch/powerpc/kernel/signal.c
+++ 2.6.23/arch/powerpc/kernel/signal.c
@@ -16,6 +16,12 @@
#include "signal.h"
+/* Log an error when sending an unhandled signal to a process. Controlled
+ * through debug.exception-trace sysctl.
+ */
+
+int show_unhandled_signals = 1;
+
/*
* Allocate space for the signal frame
*/
^ permalink raw reply
* Re: [PATCH 4/4] Inline u3msi_compose_msi_msg()
From: Benjamin Herrenschmidt @ 2007-10-02 5:24 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <19ac8b97e98274582fe5bde914883c3585f20082.1190270165.git.michael@ellerman.id.au>
On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> In the MPIC U3 MSI code, we call u3msi_compose_msi_msg() once for each MSI.
> This is overkill, as the address is per pci device, not per MSI. So setup
> the address once, and just set the data per MSI.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/sysdev/mpic_u3msi.c | 24 +++++++++---------------
> 1 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
> index 4e50d1c..027fe01 100644
> --- a/arch/powerpc/sysdev/mpic_u3msi.c
> +++ b/arch/powerpc/sysdev/mpic_u3msi.c
> @@ -107,26 +107,17 @@ static void u3msi_teardown_msi_irqs(struct pci_dev *pdev)
> return;
> }
>
> -static void u3msi_compose_msi_msg(struct pci_dev *pdev, int virq,
> - struct msi_msg *msg)
> -{
> - u64 addr;
> -
> - addr = find_ht_magic_addr(pdev);
> - msg->address_lo = addr & 0xFFFFFFFF;
> - msg->address_hi = addr >> 32;
> - msg->data = virq_to_hw(virq);
> -
> - pr_debug("u3msi: allocated virq 0x%x (hw 0x%lx) at address 0x%lx\n",
> - virq, virq_to_hw(virq), addr);
> -}
> -
> 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;
> +
> + 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) {
> hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
> @@ -146,7 +137,10 @@ 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);
>
> - u3msi_compose_msi_msg(pdev, virq, &msg);
> + pr_debug("u3msi: allocated virq 0x%x (hw 0x%lx) addr 0x%lx\n",
> + virq, hwirq, addr);
> +
> + msg.data = hwirq;
> write_msi_msg(virq, &msg);
>
> hwirq++;
^ permalink raw reply
* Re: [PATCH 3/4] Simplify rtas_change_msi() error semantics
From: Benjamin Herrenschmidt @ 2007-10-02 5:23 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <fbd88f287ba9d9ca41b5c8da734afd595ae25922.1190270165.git.michael@ellerman.id.au>
On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> Currently rtas_change_msi() returns either the error code from RTAS, or if
> the RTAS call succeeded the number of irqs that were configured by RTAS.
> This makes checking the return value more complicated than it needs to be.
>
> Instead, have rtas_change_msi() check that the number of irqs configured by
> RTAS is equal to what we requested - and return an error otherwise. This makes
> the return semantics match the usual 0 for success, something else for error.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Looks allright, just a question tho... what do we do if it fails ? Do we
try to fallback to a lower number of MSIs ? Or what ? Dead device ?
Ben.
> ---
> arch/powerpc/platforms/pseries/msi.c | 18 +++++++++++-------
> 1 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index 9c3bcfe..2793a1b 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -70,11 +70,15 @@ static int rtas_change_msi(struct pci_dn *pdn, u32 func, u32 num_irqs)
> seq_num = rtas_ret[1];
> } while (rtas_busy_delay(rc));
>
> - if (rc == 0) /* Success */
> - rc = rtas_ret[0];
> + /*
> + * If the RTAS call succeeded, check the number of irqs is actually
> + * what we asked for. If not, return an error.
> + */
> + if (rc == 0 && rtas_ret[0] != num_irqs)
> + rc = -ENOSPC;
>
> - pr_debug("rtas_msi: ibm,change_msi(func=%d,num=%d) = (%d)\n",
> - func, num_irqs, rc);
> + pr_debug("rtas_msi: ibm,change_msi(func=%d,num=%d), got %d rc = %d\n",
> + func, num_irqs, rtas_ret[0], rc);
>
> return rc;
> }
> @@ -87,7 +91,7 @@ static void rtas_disable_msi(struct pci_dev *pdev)
> if (!pdn)
> return;
>
> - if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0) != 0)
> + if (rtas_change_msi(pdn, RTAS_CHANGE_FN, 0))
> pr_debug("rtas_msi: Setting MSIs to 0 failed!\n");
> }
>
> @@ -180,14 +184,14 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> if (type == PCI_CAP_ID_MSI) {
> rc = rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, nvec);
>
> - if (rc != nvec) {
> + if (rc) {
> pr_debug("rtas_msi: trying the old firmware call.\n");
> rc = rtas_change_msi(pdn, RTAS_CHANGE_FN, nvec);
> }
> } else
> rc = rtas_change_msi(pdn, RTAS_CHANGE_MSIX_FN, nvec);
>
> - if (rc != nvec) {
> + if (rc) {
> pr_debug("rtas_msi: rtas_change_msi() failed\n");
> return rc;
> }
^ permalink raw reply
* Re: [PATCH 2/4] Simplify error logic in rtas_setup_msi_irqs()
From: Benjamin Herrenschmidt @ 2007-10-02 5:21 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <9a0ebd42c2e31f6a99a1028dd370f9ee36c771f3.1190270165.git.michael@ellerman.id.au>
On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> rtas_setup_msi_irqs() doesn't need to call teardown() itself,
> the generic code will do this for us as long as we return a non
> zero value.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/platforms/pseries/msi.c | 17 +++--------------
> 1 files changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index 6063ea2..9c3bcfe 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -189,29 +189,22 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>
> if (rc != nvec) {
> pr_debug("rtas_msi: rtas_change_msi() failed\n");
> -
> - /*
> - * In case of an error it's not clear whether the device is
> - * left with MSI enabled or not, so we explicitly disable.
> - */
> - goto out_free;
> + return rc;
> }
>
> i = 0;
> list_for_each_entry(entry, &pdev->msi_list, list) {
> hwirq = rtas_query_irq_number(pdn, i);
> if (hwirq < 0) {
> - rc = hwirq;
> pr_debug("rtas_msi: error (%d) getting hwirq\n", rc);
> - goto out_free;
> + return hwirq;
> }
>
> virq = irq_create_mapping(NULL, hwirq);
>
> if (virq == NO_IRQ) {
> pr_debug("rtas_msi: Failed mapping hwirq %d\n", hwirq);
> - rc = -ENOSPC;
> - goto out_free;
> + return -ENOSPC;
> }
>
> dev_dbg(&pdev->dev, "rtas_msi: allocated virq %d\n", virq);
> @@ -220,10 +213,6 @@ static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> }
>
> return 0;
> -
> - out_free:
> - rtas_teardown_msi_irqs(pdev);
> - return rc;
> }
>
> static void rtas_msi_pci_irq_fixup(struct pci_dev *pdev)
^ permalink raw reply
* Re: [PATCH 1/4] Simplify error logic in u3msi_setup_msi_irqs()
From: Benjamin Herrenschmidt @ 2007-10-02 5:21 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <3034ec8fd939bd5cfcdb7ac65206ae2771dc9b2c.1190270165.git.michael@ellerman.id.au>
On Thu, 2007-09-20 at 16:36 +1000, Michael Ellerman wrote:
> u3msi_setup_msi_irqs() doesn't need to call teardown() itself,
> the generic code will do this for us as long as we return a non
> zero value.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/sysdev/mpic_u3msi.c | 11 ++---------
> 1 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
> index 305b864..4e50d1c 100644
> --- a/arch/powerpc/sysdev/mpic_u3msi.c
> +++ b/arch/powerpc/sysdev/mpic_u3msi.c
> @@ -124,7 +124,6 @@ static void u3msi_compose_msi_msg(struct pci_dev *pdev, int virq,
> static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> {
> irq_hw_number_t hwirq;
> - int rc;
> unsigned int virq;
> struct msi_desc *entry;
> struct msi_msg msg;
> @@ -132,17 +131,15 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> list_for_each_entry(entry, &pdev->msi_list, list) {
> hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
> if (hwirq < 0) {
> - rc = hwirq;
> pr_debug("u3msi: failed allocating hwirq\n");
> - goto out_free;
> + return hwirq;
> }
>
> 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);
> - rc = -ENOSPC;
> - goto out_free;
> + return -ENOSPC;
> }
>
> set_irq_msi(virq, entry);
> @@ -156,10 +153,6 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> }
>
> return 0;
> -
> - out_free:
> - u3msi_teardown_msi_irqs(pdev);
> - return rc;
> }
>
> int mpic_u3msi_init(struct mpic *mpic)
^ permalink raw reply
* Re: [PATCH 7/7] Remove msic_dcr_read() and use dcr_map_reg() in axon_msi.c
From: Benjamin Herrenschmidt @ 2007-10-02 5:20 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <f2d1c6be4dd138a847c2573ae1cde5c2ac253dfa.1190009070.git.michael@ellerman.id.au>
On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> msic_dcr_read() doesn't really do anything useful, just replace it with
> direct calls to dcr_read().
>
> Use dcr_map_reg() in the axon_msi setup code, rather than essentially doing
> it by hand.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/platforms/cell/axon_msi.c | 22 +++-------------------
> 1 files changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
> index 26a5e88..57a6149 100644
> --- a/arch/powerpc/platforms/cell/axon_msi.c
> +++ b/arch/powerpc/platforms/cell/axon_msi.c
> @@ -80,18 +80,13 @@ static void msic_dcr_write(struct axon_msic *msic, unsigned int dcr_n, u32 val)
> dcr_write(msic->dcr_host, dcr_n, val);
> }
>
> -static u32 msic_dcr_read(struct axon_msic *msic, unsigned int dcr_n)
> -{
> - return dcr_read(msic->dcr_host, dcr_n);
> -}
> -
> static void axon_msi_cascade(unsigned int irq, struct irq_desc *desc)
> {
> struct axon_msic *msic = get_irq_data(irq);
> u32 write_offset, msi;
> int idx;
>
> - write_offset = msic_dcr_read(msic, MSIC_WRITE_OFFSET_REG);
> + write_offset = dcr_read(msic->dcr_host, MSIC_WRITE_OFFSET_REG);
> pr_debug("axon_msi: original write_offset 0x%x\n", write_offset);
>
> /* write_offset doesn't wrap properly, so we have to mask it */
> @@ -306,7 +301,7 @@ static int axon_msi_notify_reboot(struct notifier_block *nb,
> list_for_each_entry(msic, &axon_msic_list, list) {
> pr_debug("axon_msi: disabling %s\n",
> msic->irq_host->of_node->full_name);
> - tmp = msic_dcr_read(msic, MSIC_CTRL_REG);
> + tmp = dcr_read(msic->dcr_host, MSIC_CTRL_REG);
> tmp &= ~MSIC_CTRL_ENABLE & ~MSIC_CTRL_IRQ_ENABLE;
> msic_dcr_write(msic, MSIC_CTRL_REG, tmp);
> }
> @@ -323,7 +318,6 @@ static int axon_msi_setup_one(struct device_node *dn)
> struct page *page;
> struct axon_msic *msic;
> unsigned int virq;
> - int dcr_base, dcr_len;
>
> pr_debug("axon_msi: setting up dn %s\n", dn->full_name);
>
> @@ -334,17 +328,7 @@ static int axon_msi_setup_one(struct device_node *dn)
> goto out;
> }
>
> - dcr_base = dcr_resource_start(dn, 0);
> - dcr_len = dcr_resource_len(dn, 0);
> -
> - if (dcr_base == 0 || dcr_len == 0) {
> - printk(KERN_ERR
> - "axon_msi: couldn't parse dcr properties on %s\n",
> - dn->full_name);
> - goto out;
> - }
> -
> - msic->dcr_host = dcr_map(dn, dcr_base, dcr_len);
> + msic->dcr_host = dcr_map_reg(dn, 0);
> if (!DCR_MAP_OK(msic->dcr_host)) {
> printk(KERN_ERR "axon_msi: dcr_map failed for %s\n",
> dn->full_name);
^ permalink raw reply
* Re: [PATCH 6/7] Add dcr_map_reg() helper
From: Benjamin Herrenschmidt @ 2007-10-02 5:19 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <60649f36e785af905b8cb1fd12847d7630e7371a.1190009070.git.michael@ellerman.id.au>
On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> Add a helper routine to map dcr's based on the "dcr-reg" property of
> a device node.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Wouldn't it be more consistent to call it of_map_dcr ? Or maybe find an
even better name, but dcr_map_reg really sucks :-)
Ben.
> ---
> arch/powerpc/sysdev/dcr.c | 17 +++++++++++++++++
> include/asm-powerpc/dcr.h | 1 +
> 2 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
> index ab11c0b..da4f9c6 100644
> --- a/arch/powerpc/sysdev/dcr.c
> +++ b/arch/powerpc/sysdev/dcr.c
> @@ -126,6 +126,23 @@ dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
> }
> EXPORT_SYMBOL_GPL(dcr_map);
>
> +dcr_host_t dcr_map_reg(struct device_node *dev, unsigned int index)
> +{
> + dcr_host_t ret = { .token = NULL };
> +
> + unsigned int dcr_n, dcr_c;
> +
> + dcr_n = dcr_resource_start(dev, index);
> + if (!dcr_n)
> + return ret;
> +
> + dcr_c = dcr_resource_len(dev, index);
> + if (!dcr_c)
> + return ret;
> +
> + return dcr_map(dev, dcr_n, dcr_c);
> +}
> +
> void dcr_unmap(dcr_host_t host, unsigned int dcr_n, unsigned int dcr_c)
> {
> dcr_host_t h = host;
> diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
> index 9338d50..4d42f01 100644
> --- a/include/asm-powerpc/dcr.h
> +++ b/include/asm-powerpc/dcr.h
> @@ -38,6 +38,7 @@ extern unsigned int dcr_resource_start(struct device_node *np,
> unsigned int index);
> extern unsigned int dcr_resource_len(struct device_node *np,
> unsigned int index);
> +extern dcr_host_t dcr_map_reg(struct device_node *np, unsigned int index);
> #endif /* CONFIG_PPC_MERGE */
>
> #endif /* CONFIG_PPC_DCR */
^ permalink raw reply
* Re: [PATCH 5/7] Add dcr_host_t.base in dcr_read()/dcr_write()
From: Benjamin Herrenschmidt @ 2007-10-02 5:17 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <0caf686b20c5a22172d41e6c77b5b0bb3c429534.1190009070.git.michael@ellerman.id.au>
On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> Now that all users of dcr_read()/dcr_write() add the dcr_host_t.base, we can
> save them the trouble and do it in dcr_read()/dcr_write().
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Please, fixup the changeset comment to be more exlicit or provide some
Documentation/powerpc/dcr.txt explaning some of the discussions we had
about why this is actually a good idea :-)
Among others:
- Initially, the goal was to operate like mfdcr/mtdcr who take absolute
DCR numbers. The reason is that on 4xx hardware, indirect DCR access is
a pain (goes through a table of instructions) and it's useful to have
the compiler resolve an absolute DCR inline.
- We decided that wasn't worth the API bastardisation since most cases
where absolute DCR values are used are low level 4xx-only code which may
as well continue using mfdcr/mtdcr, while the new API is designed for
device "instances" that can exist on 4xx and Axon type platforms and may
be located at variable DCR offsets.
Something around those lines...
Appart from that, patch is fine, I'll ack with the new comment :-)
Ben.
> ---
> arch/powerpc/platforms/cell/axon_msi.c | 4 ++--
> arch/powerpc/sysdev/mpic.c | 4 ++--
> drivers/net/ibm_emac/ibm_emac_mal.h | 4 ++--
> include/asm-powerpc/dcr-mmio.h | 4 ++--
> include/asm-powerpc/dcr-native.h | 4 ++--
> 5 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
> index 23e039a..26a5e88 100644
> --- a/arch/powerpc/platforms/cell/axon_msi.c
> +++ b/arch/powerpc/platforms/cell/axon_msi.c
> @@ -77,12 +77,12 @@ static void msic_dcr_write(struct axon_msic *msic, unsigned int dcr_n, u32 val)
> {
> pr_debug("axon_msi: dcr_write(0x%x, 0x%x)\n", val, dcr_n);
>
> - dcr_write(msic->dcr_host, msic->dcr_host.base + dcr_n, val);
> + dcr_write(msic->dcr_host, dcr_n, val);
> }
>
> static u32 msic_dcr_read(struct axon_msic *msic, unsigned int dcr_n)
> {
> - return dcr_read(msic->dcr_host, msic->dcr_host.base + dcr_n);
> + return dcr_read(msic->dcr_host, dcr_n);
> }
>
> static void axon_msi_cascade(unsigned int irq, struct irq_desc *desc)
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 16b1f4b..61f5730 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -156,7 +156,7 @@ static inline u32 _mpic_read(enum mpic_reg_type type,
> switch(type) {
> #ifdef CONFIG_PPC_DCR
> case mpic_access_dcr:
> - return dcr_read(rb->dhost, rb->dhost.base + reg);
> + return dcr_read(rb->dhost, reg);
> #endif
> case mpic_access_mmio_be:
> return in_be32(rb->base + (reg >> 2));
> @@ -173,7 +173,7 @@ static inline void _mpic_write(enum mpic_reg_type type,
> switch(type) {
> #ifdef CONFIG_PPC_DCR
> case mpic_access_dcr:
> - return dcr_write(rb->dhost, rb->dhost.base + reg, value);
> + return dcr_write(rb->dhost, reg, value);
> #endif
> case mpic_access_mmio_be:
> return out_be32(rb->base + (reg >> 2), value);
> diff --git a/drivers/net/ibm_emac/ibm_emac_mal.h b/drivers/net/ibm_emac/ibm_emac_mal.h
> index 6b1fbeb..10dc978 100644
> --- a/drivers/net/ibm_emac/ibm_emac_mal.h
> +++ b/drivers/net/ibm_emac/ibm_emac_mal.h
> @@ -208,12 +208,12 @@ struct ibm_ocp_mal {
>
> static inline u32 get_mal_dcrn(struct ibm_ocp_mal *mal, int reg)
> {
> - return dcr_read(mal->dcrhost, mal->dcrhost.base + reg);
> + return dcr_read(mal->dcrhost, reg);
> }
>
> static inline void set_mal_dcrn(struct ibm_ocp_mal *mal, int reg, u32 val)
> {
> - dcr_write(mal->dcrhost, mal->dcrhost.base + reg, val);
> + dcr_write(mal->dcrhost, reg, val);
> }
>
> /* Register MAL devices */
> diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
> index 6b82c3b..a7d9eaf 100644
> --- a/include/asm-powerpc/dcr-mmio.h
> +++ b/include/asm-powerpc/dcr-mmio.h
> @@ -37,12 +37,12 @@ extern void dcr_unmap(dcr_host_t host, unsigned int dcr_n, unsigned int dcr_c);
>
> static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
> {
> - return in_be32(host.token + dcr_n * host.stride);
> + return in_be32(host.token + ((host.base + dcr_n) * host.stride));
> }
>
> static inline void dcr_write(dcr_host_t host, unsigned int dcr_n, u32 value)
> {
> - out_be32(host.token + dcr_n * host.stride, value);
> + out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
> }
>
> extern u64 of_translate_dcr_address(struct device_node *dev,
> diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
> index f41058c..3bc780f 100644
> --- a/include/asm-powerpc/dcr-native.h
> +++ b/include/asm-powerpc/dcr-native.h
> @@ -30,8 +30,8 @@ typedef struct {
>
> #define dcr_map(dev, dcr_n, dcr_c) ((dcr_host_t){ .base = (dcr_n) })
> #define dcr_unmap(host, dcr_n, dcr_c) do {} while (0)
> -#define dcr_read(host, dcr_n) mfdcr(dcr_n)
> -#define dcr_write(host, dcr_n, value) mtdcr(dcr_n, value)
> +#define dcr_read(host, dcr_n) mfdcr(dcr_n + host.base)
> +#define dcr_write(host, dcr_n, value) mtdcr(dcr_n + host.base, value)
>
> /* Device Control Registers */
> void __mtdcr(int reg, unsigned int val);
> --
> 1.5.1.3.g7a33b
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH 4/7] Update axon_msi to use dcr_host_t.base
From: Benjamin Herrenschmidt @ 2007-10-02 5:14 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <2a6b3b82775119b4a1e1fd9b290051b1fef66d55.1190009070.git.michael@ellerman.id.au>
On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> Now that dcr_host_t contains the base address, we can use that in the
> axon_msi code, rather than storing it separately.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/platforms/cell/axon_msi.c | 13 ++++++-------
> 1 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
> index 74407af..23e039a 100644
> --- a/arch/powerpc/platforms/cell/axon_msi.c
> +++ b/arch/powerpc/platforms/cell/axon_msi.c
> @@ -69,7 +69,6 @@ struct axon_msic {
> dcr_host_t dcr_host;
> struct list_head list;
> u32 read_offset;
> - u32 dcr_base;
> };
>
> static LIST_HEAD(axon_msic_list);
> @@ -78,12 +77,12 @@ static void msic_dcr_write(struct axon_msic *msic, unsigned int dcr_n, u32 val)
> {
> pr_debug("axon_msi: dcr_write(0x%x, 0x%x)\n", val, dcr_n);
>
> - dcr_write(msic->dcr_host, msic->dcr_base + dcr_n, val);
> + dcr_write(msic->dcr_host, msic->dcr_host.base + dcr_n, val);
> }
>
> static u32 msic_dcr_read(struct axon_msic *msic, unsigned int dcr_n)
> {
> - return dcr_read(msic->dcr_host, msic->dcr_base + dcr_n);
> + return dcr_read(msic->dcr_host, msic->dcr_host.base + dcr_n);
> }
>
> static void axon_msi_cascade(unsigned int irq, struct irq_desc *desc)
> @@ -324,7 +323,7 @@ static int axon_msi_setup_one(struct device_node *dn)
> struct page *page;
> struct axon_msic *msic;
> unsigned int virq;
> - int dcr_len;
> + int dcr_base, dcr_len;
>
> pr_debug("axon_msi: setting up dn %s\n", dn->full_name);
>
> @@ -335,17 +334,17 @@ static int axon_msi_setup_one(struct device_node *dn)
> goto out;
> }
>
> - msic->dcr_base = dcr_resource_start(dn, 0);
> + dcr_base = dcr_resource_start(dn, 0);
> dcr_len = dcr_resource_len(dn, 0);
>
> - if (msic->dcr_base == 0 || dcr_len == 0) {
> + if (dcr_base == 0 || dcr_len == 0) {
> printk(KERN_ERR
> "axon_msi: couldn't parse dcr properties on %s\n",
> dn->full_name);
> goto out;
> }
>
> - msic->dcr_host = dcr_map(dn, msic->dcr_base, dcr_len);
> + msic->dcr_host = dcr_map(dn, dcr_base, dcr_len);
> if (!DCR_MAP_OK(msic->dcr_host)) {
> printk(KERN_ERR "axon_msi: dcr_map failed for %s\n",
> dn->full_name);
^ permalink raw reply
* Re: [PATCH 3/7] Use dcr_host_t.base in ibm_emac_mal
From: Benjamin Herrenschmidt @ 2007-10-02 5:13 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <5022a72815fe6334b5255dbe1288036131c2c961.1190009070.git.michael@ellerman.id.au>
On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> This requires us to do a sort-of fake dcr_map(), so that base is set
> properly. This will be fixed/removed when the device-tree-aware emac driver
> is merged.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
(Have you actually tested btw ? :-)
> ---
> drivers/net/ibm_emac/ibm_emac_mal.c | 5 ++++-
> drivers/net/ibm_emac/ibm_emac_mal.h | 5 ++---
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ibm_emac/ibm_emac_mal.c b/drivers/net/ibm_emac/ibm_emac_mal.c
> index cabd984..b08da96 100644
> --- a/drivers/net/ibm_emac/ibm_emac_mal.c
> +++ b/drivers/net/ibm_emac/ibm_emac_mal.c
> @@ -421,7 +421,10 @@ static int __init mal_probe(struct ocp_device *ocpdev)
> ocpdev->def->index);
> return -ENOMEM;
> }
> - mal->dcrbase = maldata->dcr_base;
> +
> + /* XXX This only works for native dcr for now */
> + mal->dcrhost = dcr_map(NULL, maldata->dcr_base, 0);
> +
> mal->def = ocpdev->def;
>
> INIT_LIST_HEAD(&mal->poll_list);
> diff --git a/drivers/net/ibm_emac/ibm_emac_mal.h b/drivers/net/ibm_emac/ibm_emac_mal.h
> index 64bc338..6b1fbeb 100644
> --- a/drivers/net/ibm_emac/ibm_emac_mal.h
> +++ b/drivers/net/ibm_emac/ibm_emac_mal.h
> @@ -191,7 +191,6 @@ struct mal_commac {
> };
>
> struct ibm_ocp_mal {
> - int dcrbase;
> dcr_host_t dcrhost;
>
> struct list_head poll_list;
> @@ -209,12 +208,12 @@ struct ibm_ocp_mal {
>
> static inline u32 get_mal_dcrn(struct ibm_ocp_mal *mal, int reg)
> {
> - return dcr_read(mal->dcrhost, mal->dcrbase + reg);
> + return dcr_read(mal->dcrhost, mal->dcrhost.base + reg);
> }
>
> static inline void set_mal_dcrn(struct ibm_ocp_mal *mal, int reg, u32 val)
> {
> - dcr_write(mal->dcrhost, mal->dcrbase + reg, val);
> + dcr_write(mal->dcrhost, mal->dcrhost.base + reg, val);
> }
>
> /* Register MAL devices */
^ permalink raw reply
* Re: [PATCH 2/7] Update mpic to use dcr_host_t.base
From: Benjamin Herrenschmidt @ 2007-10-02 5:12 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <59ba66469e13b37eddf4e2423ed0540cce2e1079.1190009070.git.michael@ellerman.id.au>
On Mon, 2007-09-17 at 16:05 +1000, Michael Ellerman wrote:
> Now that dcr_host_t contains the base address, we can use that in the mpic
> code, rather than storing it separately.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/sysdev/mpic.c | 28 +++++++++++-----------------
> include/asm-powerpc/mpic.h | 6 ------
> 2 files changed, 11 insertions(+), 23 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 8de29f2..16b1f4b 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -156,8 +156,7 @@ static inline u32 _mpic_read(enum mpic_reg_type type,
> switch(type) {
> #ifdef CONFIG_PPC_DCR
> case mpic_access_dcr:
> - return dcr_read(rb->dhost,
> - rb->dbase + reg + rb->doff);
> + return dcr_read(rb->dhost, rb->dhost.base + reg);
> #endif
> case mpic_access_mmio_be:
> return in_be32(rb->base + (reg >> 2));
> @@ -174,8 +173,7 @@ static inline void _mpic_write(enum mpic_reg_type type,
> switch(type) {
> #ifdef CONFIG_PPC_DCR
> case mpic_access_dcr:
> - return dcr_write(rb->dhost,
> - rb->dbase + reg + rb->doff, value);
> + return dcr_write(rb->dhost, rb->dhost.base + reg, value);
> #endif
> case mpic_access_mmio_be:
> return out_be32(rb->base + (reg >> 2), value);
> @@ -279,9 +277,11 @@ static void _mpic_map_mmio(struct mpic *mpic, unsigned long phys_addr,
> static void _mpic_map_dcr(struct mpic *mpic, struct mpic_reg_bank *rb,
> unsigned int offset, unsigned int size)
> {
> - rb->dbase = mpic->dcr_base;
> - rb->doff = offset;
> - rb->dhost = dcr_map(mpic->irqhost->of_node, rb->dbase + rb->doff, size);
> + const u32 *dbasep;
> +
> + dbasep = of_get_property(mpic->irqhost->of_node, "dcr-reg", NULL);
> +
> + rb->dhost = dcr_map(mpic->irqhost->of_node, *dbasep + offset, size);
> BUG_ON(!DCR_MAP_OK(rb->dhost));
> }
>
> @@ -1075,20 +1075,14 @@ struct mpic * __init mpic_alloc(struct device_node *node,
> BUG_ON(paddr == 0 && node == NULL);
>
> /* If no physical address passed in, check if it's dcr based */
> - if (paddr == 0 && of_get_property(node, "dcr-reg", NULL) != NULL)
> - mpic->flags |= MPIC_USES_DCR;
> -
> + if (paddr == 0 && of_get_property(node, "dcr-reg", NULL) != NULL) {
> #ifdef CONFIG_PPC_DCR
> - if (mpic->flags & MPIC_USES_DCR) {
> - const u32 *dbasep;
> - dbasep = of_get_property(node, "dcr-reg", NULL);
> - BUG_ON(dbasep == NULL);
> - mpic->dcr_base = *dbasep;
> + mpic->flags |= MPIC_USES_DCR;
> mpic->reg_type = mpic_access_dcr;
> - }
> #else
> - BUG_ON (mpic->flags & MPIC_USES_DCR);
> + BUG();
> #endif /* CONFIG_PPC_DCR */
> + }
>
> /* If the MPIC is not DCR based, and no physical address was passed
> * in, try to obtain one
> diff --git a/include/asm-powerpc/mpic.h b/include/asm-powerpc/mpic.h
> index edb4a7c..ae84dde 100644
> --- a/include/asm-powerpc/mpic.h
> +++ b/include/asm-powerpc/mpic.h
> @@ -224,8 +224,6 @@ struct mpic_reg_bank {
> u32 __iomem *base;
> #ifdef CONFIG_PPC_DCR
> dcr_host_t dhost;
> - unsigned int dbase;
> - unsigned int doff;
> #endif /* CONFIG_PPC_DCR */
> };
>
> @@ -289,10 +287,6 @@ struct mpic
> struct mpic_reg_bank cpuregs[MPIC_MAX_CPUS];
> struct mpic_reg_bank isus[MPIC_MAX_ISU];
>
> -#ifdef CONFIG_PPC_DCR
> - unsigned int dcr_base;
> -#endif
> -
> /* Protected sources */
> unsigned long *protected;
>
^ 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