Re: [PATCH 4/4] scsi: pm8001: fix pm8001_store_update_fw

2014-07-30 Thread Tomas Henzl 
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

2014-07-10 Thread Christoph Hellwig 
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

Re: [PATCH 4/4] scsi: pm8001: fix pm8001_store_update_fw

2014-07-10 Thread Tomas Henzl 
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

[PATCH 4/4] scsi: pm8001: fix pm8001_store_update_fw

2014-07-07 Thread Tomas Henzl 
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 the...@redhat.com
---
 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