* [PATCH 6/6] pm80xx : Fixed return value issue
@ 2013-11-20 7:22 Viswas G
2013-11-20 9:37 ` Jack Wang
2013-11-20 16:46 ` Tomas Henzl
0 siblings, 2 replies; 6+ messages in thread
From: Viswas G @ 2013-11-20 7:22 UTC (permalink / raw)
To: linux-scsi
Cc: xjtuwjp, Vasanthalakshmi.Tharmarajan, Suresh.Thiagarajan,
Viswas.G
pm8001_get_gsm_dump() was returning "1" in error case
instead of negative error code.
Signed-off-by: Viswas G <Viswas.G@pmcs.com>
---
drivers/scsi/pm8001/pm8001_hwi.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index f6ea277..e2932ef 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -5020,7 +5020,7 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
/* check max is 1 Mbytes */
if ((length > 0x100000) || (gsm_dump_offset & 3) ||
((gsm_dump_offset + length) > 0x1000000))
- return 1;
+ return -EINVAL;
if (pm8001_ha->chip_id == chip_8001)
bar = 2;
@@ -5048,12 +5048,12 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
gsm_base = GSM_BASE;
if (-1 == pm8001_bar4_shift(pm8001_ha,
(gsm_base + shift_value)))
- return 1;
+ return -EINVAL;
} else {
gsm_base = 0;
if (-1 == pm80xx_bar4_shift(pm8001_ha,
(gsm_base + shift_value)))
- return 1;
+ return -EINVAL;
}
gsm_dump_offset = (gsm_dump_offset + offset) &
0xFFFF0000;
@@ -5073,7 +5073,7 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
}
/* Shift back to BAR4 original address */
if (-1 == pm8001_bar4_shift(pm8001_ha, 0))
- return 1;
+ return -EINVAL;
pm8001_ha->fatal_forensic_shift_offset += 1024;
if (pm8001_ha->fatal_forensic_shift_offset >= 0x100000)
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 6/6] pm80xx : Fixed return value issue
2013-11-20 7:22 [PATCH 6/6] pm80xx : Fixed return value issue Viswas G
@ 2013-11-20 9:37 ` Jack Wang
2013-11-20 16:46 ` Tomas Henzl
1 sibling, 0 replies; 6+ messages in thread
From: Jack Wang @ 2013-11-20 9:37 UTC (permalink / raw)
To: Viswas G; +Cc: linux-scsi, Vasanthalakshmi.Tharmarajan, Suresh.Thiagarajan
On 11/20/2013 08:22 AM, Viswas G wrote:
> pm8001_get_gsm_dump() was returning "1" in error case
> instead of negative error code.
>
> Signed-off-by: Viswas G <Viswas.G@pmcs.com>
Looks good, thanks
Reviewed-by: Jack Wang <jinpu.wang@profitbricks.com>
> ---
> drivers/scsi/pm8001/pm8001_hwi.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
> index f6ea277..e2932ef 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -5020,7 +5020,7 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
> /* check max is 1 Mbytes */
> if ((length > 0x100000) || (gsm_dump_offset & 3) ||
> ((gsm_dump_offset + length) > 0x1000000))
> - return 1;
> + return -EINVAL;
>
> if (pm8001_ha->chip_id == chip_8001)
> bar = 2;
> @@ -5048,12 +5048,12 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
> gsm_base = GSM_BASE;
> if (-1 == pm8001_bar4_shift(pm8001_ha,
> (gsm_base + shift_value)))
> - return 1;
> + return -EINVAL;
> } else {
> gsm_base = 0;
> if (-1 == pm80xx_bar4_shift(pm8001_ha,
> (gsm_base + shift_value)))
> - return 1;
> + return -EINVAL;
> }
> gsm_dump_offset = (gsm_dump_offset + offset) &
> 0xFFFF0000;
> @@ -5073,7 +5073,7 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
> }
> /* Shift back to BAR4 original address */
> if (-1 == pm8001_bar4_shift(pm8001_ha, 0))
> - return 1;
> + return -EINVAL;
> pm8001_ha->fatal_forensic_shift_offset += 1024;
>
> if (pm8001_ha->fatal_forensic_shift_offset >= 0x100000)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 6/6] pm80xx : Fixed return value issue
2013-11-20 7:22 [PATCH 6/6] pm80xx : Fixed return value issue Viswas G
2013-11-20 9:37 ` Jack Wang
@ 2013-11-20 16:46 ` Tomas Henzl
2013-11-21 3:56 ` Viswas G
1 sibling, 1 reply; 6+ messages in thread
From: Tomas Henzl @ 2013-11-20 16:46 UTC (permalink / raw)
To: Viswas G, linux-scsi
Cc: xjtuwjp, Vasanthalakshmi.Tharmarajan, Suresh.Thiagarajan
On 11/20/2013 08:22 AM, Viswas G wrote:
> pm8001_get_gsm_dump() was returning "1" in error case
> instead of negative error code.
Hi,
the pm8001_get_gsm_dum return value is used here I think:
----------------
static ssize_t pm8001_ctl_gsm_log_show(struct device *cdev,
struct device_attribute *attr, char *buf)
{
u32 count;
count = pm8001_get_gsm_dump(cdev, SYSFS_OFFSET, buf);
---------------
could you also change the 'count' from unsigned to a signed type?
Maybe in some cases another errno would make more sense (ENODEV, EIO?),
but I'll leave this up to you.
I have noticed in this series only patches 5/6 and 6/6, I miss the
1-4/6, could you help me find them ? What are the subjects of 1-4/6?
Thanks, Tomas
>
> Signed-off-by: Viswas G <Viswas.G@pmcs.com>
> ---
> drivers/scsi/pm8001/pm8001_hwi.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
> index f6ea277..e2932ef 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -5020,7 +5020,7 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
> /* check max is 1 Mbytes */
> if ((length > 0x100000) || (gsm_dump_offset & 3) ||
> ((gsm_dump_offset + length) > 0x1000000))
> - return 1;
> + return -EINVAL;
>
> if (pm8001_ha->chip_id == chip_8001)
> bar = 2;
> @@ -5048,12 +5048,12 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
> gsm_base = GSM_BASE;
> if (-1 == pm8001_bar4_shift(pm8001_ha,
> (gsm_base + shift_value)))
> - return 1;
> + return -EINVAL;
> } else {
> gsm_base = 0;
> if (-1 == pm80xx_bar4_shift(pm8001_ha,
> (gsm_base + shift_value)))
> - return 1;
> + return -EINVAL;
> }
> gsm_dump_offset = (gsm_dump_offset + offset) &
> 0xFFFF0000;
> @@ -5073,7 +5073,7 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
> }
> /* Shift back to BAR4 original address */
> if (-1 == pm8001_bar4_shift(pm8001_ha, 0))
> - return 1;
> + return -EINVAL;
> pm8001_ha->fatal_forensic_shift_offset += 1024;
>
> if (pm8001_ha->fatal_forensic_shift_offset >= 0x100000)
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 6/6] pm80xx : Fixed return value issue
2013-11-20 16:46 ` Tomas Henzl
@ 2013-11-21 3:56 ` Viswas G
2013-11-21 12:32 ` Tomas Henzl
0 siblings, 1 reply; 6+ messages in thread
From: Viswas G @ 2013-11-21 3:56 UTC (permalink / raw)
To: Tomas Henzl, linux-scsi@vger.kernel.org
Cc: xjtuwjp@gmail.com, Vasanthalakshmi Tharmarajan,
Suresh Thiagarajan
Hi Tomas,
Patches 1-4 are submitted with subject "[PATCH V2 0/4]: pm8001 driver bug fixes".
As this patch has to be applied after those four patches, we submitted this as 5/6 and 6/6.
We will resend the modified patch.
Regards,
Viswas G
-----Original Message-----
From: Tomas Henzl [mailto:thenzl@redhat.com]
Sent: Wednesday, November 20, 2013 10:16 PM
To: Viswas G; linux-scsi@vger.kernel.org
Cc: xjtuwjp@gmail.com; Vasanthalakshmi Tharmarajan; Suresh Thiagarajan
Subject: Re: [PATCH 6/6] pm80xx : Fixed return value issue
On 11/20/2013 08:22 AM, Viswas G wrote:
> pm8001_get_gsm_dump() was returning "1" in error case instead of
> negative error code.
Hi,
the pm8001_get_gsm_dum return value is used here I think:
----------------
static ssize_t pm8001_ctl_gsm_log_show(struct device *cdev,
struct device_attribute *attr, char *buf) {
u32 count;
count = pm8001_get_gsm_dump(cdev, SYSFS_OFFSET, buf);
---------------
could you also change the 'count' from unsigned to a signed type?
Maybe in some cases another errno would make more sense (ENODEV, EIO?), but I'll leave this up to you.
I have noticed in this series only patches 5/6 and 6/6, I miss the 1-4/6, could you help me find them ? What are the subjects of 1-4/6?
Thanks, Tomas
>
> Signed-off-by: Viswas G <Viswas.G@pmcs.com>
> ---
> drivers/scsi/pm8001/pm8001_hwi.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c
> b/drivers/scsi/pm8001/pm8001_hwi.c
> index f6ea277..e2932ef 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -5020,7 +5020,7 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
> /* check max is 1 Mbytes */
> if ((length > 0x100000) || (gsm_dump_offset & 3) ||
> ((gsm_dump_offset + length) > 0x1000000))
> - return 1;
> + return -EINVAL;
>
> if (pm8001_ha->chip_id == chip_8001)
> bar = 2;
> @@ -5048,12 +5048,12 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
> gsm_base = GSM_BASE;
> if (-1 == pm8001_bar4_shift(pm8001_ha,
> (gsm_base + shift_value)))
> - return 1;
> + return -EINVAL;
> } else {
> gsm_base = 0;
> if (-1 == pm80xx_bar4_shift(pm8001_ha,
> (gsm_base + shift_value)))
> - return 1;
> + return -EINVAL;
> }
> gsm_dump_offset = (gsm_dump_offset + offset) &
> 0xFFFF0000;
> @@ -5073,7 +5073,7 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
> }
> /* Shift back to BAR4 original address */
> if (-1 == pm8001_bar4_shift(pm8001_ha, 0))
> - return 1;
> + return -EINVAL;
> pm8001_ha->fatal_forensic_shift_offset += 1024;
>
> if (pm8001_ha->fatal_forensic_shift_offset >= 0x100000)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 6/6] pm80xx : Fixed return value issue
2013-11-21 3:56 ` Viswas G
@ 2013-11-21 12:32 ` Tomas Henzl
2013-11-21 15:25 ` James Bottomley
0 siblings, 1 reply; 6+ messages in thread
From: Tomas Henzl @ 2013-11-21 12:32 UTC (permalink / raw)
To: Viswas G, linux-scsi@vger.kernel.org
Cc: xjtuwjp@gmail.com, Vasanthalakshmi Tharmarajan,
Suresh Thiagarajan
On 11/21/2013 04:56 AM, Viswas G wrote:
> Hi Tomas,
>
> Patches 1-4 are submitted with subject "[PATCH V2 0/4]: pm8001 driver bug fixes".
>
> As this patch has to be applied after those four patches, we submitted this as 5/6 and 6/6.
I was confused because the series 1-4/4 was send by another person,
thanks for clarification.
>
> We will resend the modified patch.
>
> Regards,
> Viswas G
>
>
> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@redhat.com]
> Sent: Wednesday, November 20, 2013 10:16 PM
> To: Viswas G; linux-scsi@vger.kernel.org
> Cc: xjtuwjp@gmail.com; Vasanthalakshmi Tharmarajan; Suresh Thiagarajan
> Subject: Re: [PATCH 6/6] pm80xx : Fixed return value issue
>
> On 11/20/2013 08:22 AM, Viswas G wrote:
>> pm8001_get_gsm_dump() was returning "1" in error case instead of
>> negative error code.
> Hi,
>
> the pm8001_get_gsm_dum return value is used here I think:
> ----------------
> static ssize_t pm8001_ctl_gsm_log_show(struct device *cdev,
> struct device_attribute *attr, char *buf) {
> u32 count;
>
> count = pm8001_get_gsm_dump(cdev, SYSFS_OFFSET, buf);
> ---------------
> could you also change the 'count' from unsigned to a signed type?
>
> Maybe in some cases another errno would make more sense (ENODEV, EIO?), but I'll leave this up to you.
>
> I have noticed in this series only patches 5/6 and 6/6, I miss the 1-4/6, could you help me find them ? What are the subjects of 1-4/6?
>
> Thanks, Tomas
>
>> Signed-off-by: Viswas G <Viswas.G@pmcs.com>
>> ---
>> drivers/scsi/pm8001/pm8001_hwi.c | 8 ++++----
>> 1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c
>> b/drivers/scsi/pm8001/pm8001_hwi.c
>> index f6ea277..e2932ef 100644
>> --- a/drivers/scsi/pm8001/pm8001_hwi.c
>> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
>> @@ -5020,7 +5020,7 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
>> /* check max is 1 Mbytes */
>> if ((length > 0x100000) || (gsm_dump_offset & 3) ||
>> ((gsm_dump_offset + length) > 0x1000000))
>> - return 1;
>> + return -EINVAL;
>>
>> if (pm8001_ha->chip_id == chip_8001)
>> bar = 2;
>> @@ -5048,12 +5048,12 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
>> gsm_base = GSM_BASE;
>> if (-1 == pm8001_bar4_shift(pm8001_ha,
>> (gsm_base + shift_value)))
>> - return 1;
>> + return -EINVAL;
>> } else {
>> gsm_base = 0;
>> if (-1 == pm80xx_bar4_shift(pm8001_ha,
>> (gsm_base + shift_value)))
>> - return 1;
>> + return -EINVAL;
>> }
>> gsm_dump_offset = (gsm_dump_offset + offset) &
>> 0xFFFF0000;
>> @@ -5073,7 +5073,7 @@ pm8001_get_gsm_dump(struct device *cdev, u32 length, char* buf)
>> }
>> /* Shift back to BAR4 original address */
>> if (-1 == pm8001_bar4_shift(pm8001_ha, 0))
>> - return 1;
>> + return -EINVAL;
>> pm8001_ha->fatal_forensic_shift_offset += 1024;
>>
>> if (pm8001_ha->fatal_forensic_shift_offset >= 0x100000)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 6/6] pm80xx : Fixed return value issue
2013-11-21 12:32 ` Tomas Henzl
@ 2013-11-21 15:25 ` James Bottomley
0 siblings, 0 replies; 6+ messages in thread
From: James Bottomley @ 2013-11-21 15:25 UTC (permalink / raw)
To: Tomas Henzl
Cc: Viswas G, linux-scsi@vger.kernel.org, xjtuwjp@gmail.com,
Vasanthalakshmi Tharmarajan, Suresh Thiagarajan
On Thu, 2013-11-21 at 13:32 +0100, Tomas Henzl wrote:
> On 11/21/2013 04:56 AM, Viswas G wrote:
> > Hi Tomas,
> >
> > Patches 1-4 are submitted with subject "[PATCH V2 0/4]: pm8001 driver bug fixes".
> >
> > As this patch has to be applied after those four patches, we submitted this as 5/6 and 6/6.
>
> I was confused because the series 1-4/4 was send by another person,
And it was labelled 1-4/4 so it's not really natural to think of 5 and 6
following on ... you weren't the only one confused.
James
> thanks for clarification.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-21 15:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-20 7:22 [PATCH 6/6] pm80xx : Fixed return value issue Viswas G
2013-11-20 9:37 ` Jack Wang
2013-11-20 16:46 ` Tomas Henzl
2013-11-21 3:56 ` Viswas G
2013-11-21 12:32 ` Tomas Henzl
2013-11-21 15:25 ` James Bottomley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).