From: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
To: linux-scsi@vger.kernel.org
Cc: Matthew Wilcox <matthew@wil.cx>
Subject: [PATCH v2] advansys: fix regression with request_firmware change
Date: Fri, 26 Mar 2010 20:05:08 -0300 [thread overview]
Message-ID: <201003262005.09121.herton@mandriva.com.br> (raw)
On newer kernels users of advansys module are reporting system hang when
trying to load it without firmware files present. After looking closely
at description on https://qa.mandriva.com/show_bug.cgi?id=53220, I think
this is related to commit "[SCSI] advansys: use request_firmware". The
problem is that after switch to request_firmware, asc_dvc->err_code
isn't being set when firmware files aren't found or loading fails.
err_code is used by the driver to judge if there was a fatal error or
not, as can be seen for example on advansys_board_found, which will only
return -ENODEV when err_code is set. Because err_code isn't being set
when request_firmware fails, this is a change of behaviour of the code
before request_firmware addition, making it continue to load and it
fails later as the firmware wasn't really loaded.
Also, error handling on advansys_board_found is fixed, because it's
buggy in the case we have an ASC_NARROW_BOARD set and failure happens on
AscInitAsc1000Driver step: it was freeing items of wrong struct in the
dvc_var union of struct asc_board, which could lead to an oops in the
case we set some of the fields in struct of narrow board as code was
choosing to always freeing wide board fields, and not everything wasn't
being freed/released properly.
Signed-off-by: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
---
drivers/scsi/advansys.c | 35 ++++++++++++++++++++++++++---------
1 files changed, 26 insertions(+), 9 deletions(-)
v2 fixes error handling in advansys_board_found, so code will not oops anymore
if firmware files are not found. users tested this change and reported it to be
ok (no more freeze or oops when firmware files are not found)
diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 22626ab..9a3c68f 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -4781,12 +4781,14 @@ static ushort AscInitAsc1000Driver(ASC_DVC_VAR *asc_dvc)
if (err) {
printk(KERN_ERR "Failed to load image \"%s\" err %d\n",
fwname, err);
+ asc_dvc->err_code |= ASC_IERR_MCODE_CHKSUM;
return err;
}
if (fw->size < 4) {
printk(KERN_ERR "Bogus length %zu in image \"%s\"\n",
fw->size, fwname);
release_firmware(fw);
+ asc_dvc->err_code |= ASC_IERR_MCODE_CHKSUM;
return -EINVAL;
}
chksum = (fw->data[3] << 24) | (fw->data[2] << 16) |
@@ -5110,12 +5112,14 @@ static int AdvInitAsc3550Driver(ADV_DVC_VAR *asc_dvc)
if (err) {
printk(KERN_ERR "Failed to load image \"%s\" err %d\n",
fwname, err);
+ asc_dvc->err_code = ASC_IERR_MCODE_CHKSUM;
return err;
}
if (fw->size < 4) {
printk(KERN_ERR "Bogus length %zu in image \"%s\"\n",
fw->size, fwname);
release_firmware(fw);
+ asc_dvc->err_code = ASC_IERR_MCODE_CHKSUM;
return -EINVAL;
}
chksum = (fw->data[3] << 24) | (fw->data[2] << 16) |
@@ -5624,12 +5628,14 @@ static int AdvInitAsc38C0800Driver(ADV_DVC_VAR *asc_dvc)
if (err) {
printk(KERN_ERR "Failed to load image \"%s\" err %d\n",
fwname, err);
+ asc_dvc->err_code = ASC_IERR_MCODE_CHKSUM;
return err;
}
if (fw->size < 4) {
printk(KERN_ERR "Bogus length %zu in image \"%s\"\n",
fw->size, fwname);
release_firmware(fw);
+ asc_dvc->err_code = ASC_IERR_MCODE_CHKSUM;
return -EINVAL;
}
chksum = (fw->data[3] << 24) | (fw->data[2] << 16) |
@@ -6124,12 +6130,14 @@ static int AdvInitAsc38C1600Driver(ADV_DVC_VAR *asc_dvc)
if (err) {
printk(KERN_ERR "Failed to load image \"%s\" err %d\n",
fwname, err);
+ asc_dvc->err_code = ASC_IERR_MCODE_CHKSUM;
return err;
}
if (fw->size < 4) {
printk(KERN_ERR "Bogus length %zu in image \"%s\"\n",
fw->size, fwname);
release_firmware(fw);
+ asc_dvc->err_code = ASC_IERR_MCODE_CHKSUM;
return -EINVAL;
}
chksum = (fw->data[3] << 24) | (fw->data[2] << 16) |
@@ -12303,7 +12311,7 @@ static int __devinit advansys_board_found(struct Scsi_Host *shost,
asc_dvc_varp->overrun_buf = kzalloc(ASC_OVERRUN_BSIZE, GFP_KERNEL);
if (!asc_dvc_varp->overrun_buf) {
ret = -ENOMEM;
- goto err_free_wide_mem;
+ goto err_free_irq;
}
warn_code = AscInitAsc1000Driver(asc_dvc_varp);
@@ -12314,28 +12322,37 @@ static int __devinit advansys_board_found(struct Scsi_Host *shost,
asc_dvc_varp->err_code);
if (asc_dvc_varp->err_code) {
ret = -ENODEV;
- kfree(asc_dvc_varp->overrun_buf);
+ goto err_free_mem;
}
}
} else {
- if (advansys_wide_init_chip(shost))
+ if (advansys_wide_init_chip(shost)) {
ret = -ENODEV;
+ goto err_free_mem;
+ }
}
- if (ret)
- goto err_free_wide_mem;
-
ASC_DBG_PRT_SCSI_HOST(2, shost);
ret = scsi_add_host(shost, boardp->dev);
if (ret)
- goto err_free_wide_mem;
+ goto err_free_mem;
scsi_scan_host(shost);
return 0;
- err_free_wide_mem:
- advansys_wide_free_mem(boardp);
+ err_free_mem:
+ if (ASC_NARROW_BOARD(boardp)) {
+ if ((asc_dvc_varp->err_code & ASC_IERR_SET_PC_ADDR) ||
+ (asc_dvc_varp->err_code & ASC_IERR_START_STOP_CHIP) ||
+ (!asc_dvc_varp->err_code))
+ dma_unmap_single(boardp->dev, asc_dvc_varp->overrun_dma,
+ ASC_OVERRUN_BSIZE, DMA_FROM_DEVICE);
+ kfree(asc_dvc_varp->overrun_buf);
+ } else {
+ advansys_wide_free_mem(boardp);
+ }
+ err_free_irq:
free_irq(boardp->irq, shost);
err_free_dma:
#ifdef CONFIG_ISA
--
1.7.0.3
next reply other threads:[~2010-03-26 23:05 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-26 23:05 Herton Ronaldo Krzesinski [this message]
2010-03-28 15:25 ` [PATCH v2] advansys: fix regression with request_firmware change James Bottomley
2010-03-29 20:36 ` Herton Ronaldo Krzesinski
2010-03-29 20:49 ` Herton Ronaldo Krzesinski
2010-03-29 21:22 ` Herton Ronaldo Krzesinski
2010-03-29 23:16 ` James Bottomley
2010-03-30 16:35 ` Herton Ronaldo Krzesinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201003262005.09121.herton@mandriva.com.br \
--to=herton@mandriva.com.br \
--cc=linux-scsi@vger.kernel.org \
--cc=matthew@wil.cx \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox