| Submitter | Stefan Tauner |
|---|---|
| Date | 2012-12-28 10:31:22 |
| Message ID | <201212281031.qBSAVMVS028572@mail2.student.tuwien.ac.at> |
| Download | mbox | patch |
| Permalink | /patch/3821/ |
| State | Superseded |
| Headers | show |
Comments
On Fri, 28 Dec 2012 18:31:22 +0800, Stefan Tauner <stefan.tauner@student.tuwien.ac.at> wrote: > On Fri, 28 Dec 2012 16:53:36 +0800 > Roy <roytam@gmail.com> wrote: > >> Sure. But isn't pcilib yells the prefix path is NULL? > > Yes, but *why* was not all that clear without the stack trace. :) > Thanks Idwer for asking for it. > > I am pretty sure that we try to undo the pci write after we tell pcilib > to shutdown - obviously a bad idea and we should... > 1. guard against this behavior, > 2. check all existing drivers that they don't do the same nonsense (I > fell for this too in the past). > > Roy, can you please try the attached patch (against 0.9.6.1 stable aka > r1564; but nothing has changed since then in that file, so you can > also use current HEAD.)? segfault is fixed confirmed. Thanks.
Patch
From 44567ecdb80a5d4bf87f87f228c855027d83945c Mon Sep 17 00:00:00 2001 From: Stefan Tauner <stefan.tauner@student.tuwien.ac.at> Date: Fri, 28 Dec 2012 11:24:45 +0100 Subject: [PATCH] Fix atahpt.c shutdown. The order of pcidev_init, register_shutdown and rpci_write_* is important! Thanks to Roy for reporting the problem (testing is apparently also important ;) Signed-off-by: Stefan Tauner <stefan.tauner@student.tuwien.ac.at> --- atahpt.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/atahpt.c b/atahpt.c index f410fe4..b71811a 100644 --- a/atahpt.c +++ b/atahpt.c @@ -72,14 +72,14 @@ int atahpt_init(void) io_base_addr = pcidev_init(PCI_BASE_ADDRESS_4, ata_hpt); + if (register_shutdown(atahpt_shutdown, NULL)) + return 1; + /* Enable flash access. */ reg32 = pci_read_long(pcidev_dev, REG_FLASH_ACCESS); reg32 |= (1 << 24); rpci_write_long(pcidev_dev, REG_FLASH_ACCESS, reg32); - if (register_shutdown(atahpt_shutdown, NULL)) - return 1; - register_par_programmer(&par_programmer_atahpt, BUS_PARALLEL); return 0; -- Kind regards, Stefan Tauner
On Fri, 28 Dec 2012 16:53:36 +0800 Roy <roytam@gmail.com> wrote: > Sure. But isn't pcilib yells the prefix path is NULL? Yes, but *why* was not all that clear without the stack trace. :) Thanks Idwer for asking for it. I am pretty sure that we try to undo the pci write after we tell pcilib to shutdown - obviously a bad idea and we should... 1. guard against this behavior, 2. check all existing drivers that they don't do the same nonsense (I fell for this too in the past). Roy, can you please try the attached patch (against 0.9.6.1 stable aka r1564; but nothing has changed since then in that file, so you can also use current HEAD.)?