Re: [PATCH 4/4] scsi: pm8001: fix pm8001_store_update_fw
On 07/10/2014 03:30 PM, Tomas Henzl wrote: > On 07/10/2014 08:43 AM, Christoph Hellwig wrote: >> On Mon, Jul 07, 2014 at 05:20:01PM +0200, Tomas Henzl wrote: >>> The current implementation may mix the negative value returned >>> from pm8001_set_nvmd with with count. -(-ENOMEM) could be interpreted >>> as bytes programmed, this patch fixes it. >> This still doesn;t look correct to me as err mixes up the driver >> internal FAIL_* codes with Linux error codes. It seems like for the >> FAIL_* codes should only go into ->fw_status and the return value >> should be a proper Linux error code. > And the fw_status might be later used to show error strings in > pm8001_show_update_fw, > if it is so it depends on the flash utility but it seems likely. > >> Funny fact: the FAIL_* / FLASH_IN_PROGRESS codes seems to be the same >> between aic94xx and pm8001. > And similar story there too - asd_store_update_bios -...- > asd_poll_flash(might return -ENOENT) > > Maybe the flash utility ignores the return value or it has never happened. > - > > I'll try to find what seems to be the most probable way and post it in few > days. Christoph, from few days it is three weeks and the patch has been just replaced by "[PATCH 1/3] pm8001: fix pm8001_store_update_fw" sent by Suresh, so you can drop this patch. (I've asked Suresh off-list to test it, I didn't want to brick my card by testing fw flashes.) Thanks, Tomas > This patch is not related to the patches 1-3/4, so just wait with this one. > > Thanks, Tomas > >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] scsi: pm8001: fix pm8001_store_update_fw
On 07/10/2014 08:43 AM, Christoph Hellwig wrote: > On Mon, Jul 07, 2014 at 05:20:01PM +0200, Tomas Henzl wrote: >> The current implementation may mix the negative value returned >> from pm8001_set_nvmd with with count. -(-ENOMEM) could be interpreted >> as bytes programmed, this patch fixes it. > This still doesn;t look correct to me as err mixes up the driver > internal FAIL_* codes with Linux error codes. It seems like for the > FAIL_* codes should only go into ->fw_status and the return value > should be a proper Linux error code. And the fw_status might be later used to show error strings in pm8001_show_update_fw, if it is so it depends on the flash utility but it seems likely. > > Funny fact: the FAIL_* / FLASH_IN_PROGRESS codes seems to be the same > between aic94xx and pm8001. And similar story there too - asd_store_update_bios -...- asd_poll_flash(might return -ENOENT) Maybe the flash utility ignores the return value or it has never happened. - I'll try to find what seems to be the most probable way and post it in few days. This patch is not related to the patches 1-3/4, so just wait with this one. Thanks, Tomas > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] scsi: pm8001: fix pm8001_store_update_fw
On Mon, Jul 07, 2014 at 05:20:01PM +0200, Tomas Henzl wrote: > The current implementation may mix the negative value returned > from pm8001_set_nvmd with with count. -(-ENOMEM) could be interpreted > as bytes programmed, this patch fixes it. This still doesn;t look correct to me as err mixes up the driver internal FAIL_* codes with Linux error codes. It seems like for the FAIL_* codes should only go into ->fw_status and the return value should be a proper Linux error code. Funny fact: the FAIL_* / FLASH_IN_PROGRESS codes seems to be the same between aic94xx and pm8001. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] scsi: pm8001: fix pm8001_store_update_fw
The current implementation may mix the negative value returned
from pm8001_set_nvmd with with count. -(-ENOMEM) could be interpreted
as bytes programmed, this patch fixes it.
Signed-off-by: Tomas Henzl
---
drivers/scsi/pm8001/pm8001_ctl.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
index 8f1f882..847755b 100644
--- a/drivers/scsi/pm8001/pm8001_ctl.c
+++ b/drivers/scsi/pm8001/pm8001_ctl.c
@@ -650,14 +650,14 @@ static ssize_t pm8001_store_update_fw(struct device *cdev,
cmd_ptr = kzalloc(count*2, GFP_KERNEL);
if (!cmd_ptr) {
- err = FAIL_OUT_MEMORY;
+ err = -FAIL_OUT_MEMORY;
goto out;
}
filename_ptr = cmd_ptr + count;
res = sscanf(buf, "%s %s", cmd_ptr, filename_ptr);
if (res != 2) {
- err = FAIL_PARAMETERS;
+ err = -FAIL_PARAMETERS;
goto out1;
}
@@ -669,12 +669,12 @@ static ssize_t pm8001_store_update_fw(struct device *cdev,
}
}
if (flash_command == FLASH_CMD_NONE) {
- err = FAIL_PARAMETERS;
+ err = -FAIL_PARAMETERS;
goto out1;
}
if (pm8001_ha->fw_status == FLASH_IN_PROGRESS) {
- err = FLASH_IN_PROGRESS;
+ err = -FLASH_IN_PROGRESS;
goto out1;
}
err = request_firmware(&pm8001_ha->fw_image,
@@ -685,7 +685,7 @@ static ssize_t pm8001_store_update_fw(struct device *cdev,
PM8001_FAIL_DBG(pm8001_ha,
pm8001_printk("Failed to load firmware image file %s,"
" error %d\n", filename_ptr, err));
- err = FAIL_OPEN_BIOS_FILE;
+ err = -FAIL_OPEN_BIOS_FILE;
goto out1;
}
@@ -700,7 +700,7 @@ static ssize_t pm8001_store_update_fw(struct device *cdev,
break;
default:
pm8001_ha->fw_status = FAIL_PARAMETERS;
- err = FAIL_PARAMETERS;
+ err = -FAIL_PARAMETERS;
break;
}
release_firmware(pm8001_ha->fw_image);
@@ -709,10 +709,10 @@ out1:
out:
pm8001_ha->fw_status = err;
- if (!err)
- return count;
- else
- return -err;
+ if (err)
+ return err;
+
+ return count;
}
static ssize_t pm8001_show_update_fw(struct device *cdev,
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
