Patchwork 2theMax RAID 100 (HPT370)

login
register
about
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

Stefan Tauner - 2012-12-28 10:31:22
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.)?
Roy - 2012-12-31 02:43:21
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