diff mbox

[v2,RESEND,17/23] pm8001: Fix invalid return when request_irq() failed

Message ID 5ecd9b80f705545c3dc85b2a4ae63a8ac3df8105.1405533479.git.agordeev@redhat.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Alexander Gordeev July 16, 2014, 6:05 p.m. UTC 
When a call to request_irq() failed pm8001_setup_msix()
still returns the success. This udate fixes the described
misbehaviour.

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Cc: xjtuwjp@gmail.com
Cc: lindar_liu@usish.com
Cc: linux-scsi@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Acked-by: Jack Wang <xjtuwjp@gmail.com>
---
 drivers/scsi/pm8001/pm8001_init.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

Comments

Tomas Henzl July 29, 2014, 1:13 p.m. UTC | #1 
Hi, 
patch looks good.

A correction in pm8001_request_irq might be added too to allow 
the INT-X path when pm8001_setup_msix fails, but that is not part 
of this fix.

Tomas

Reviewed-by: Tomas Henzl <thenzl@redhat.com>

On 07/16/2014 08:05 PM, Alexander Gordeev wrote:
> When a call to request_irq() failed pm8001_setup_msix()
> still returns the success. This udate fixes the described
> misbehaviour.
>
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> Cc: xjtuwjp@gmail.com
> Cc: lindar_liu@usish.com
> Cc: linux-scsi@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> Acked-by: Jack Wang <xjtuwjp@gmail.com>
> ---
>  drivers/scsi/pm8001/pm8001_init.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index e90c89f..e837ece 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -744,9 +744,10 @@ static u32 pm8001_setup_msix(struct pm8001_hba_info *pm8001_ha)
>  			pm8001_ha->irq_vector[i].irq_id = i;
>  			pm8001_ha->irq_vector[i].drv_inst = pm8001_ha;
>  
> -			if (request_irq(pm8001_ha->msix_entries[i].vector,
> +			rc = request_irq(pm8001_ha->msix_entries[i].vector,
>  				pm8001_interrupt_handler_msix, flag,
> -				intr_drvname[i], &(pm8001_ha->irq_vector[i]))) {
> +				intr_drvname[i], &(pm8001_ha->irq_vector[i]));
> +			if (rc) {
>  				for (j = 0; j < i; j++)
>  					free_irq(
>  					pm8001_ha->msix_entries[j].vector,

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig July 29, 2014, 1:20 p.m. UTC | #2 
On Tue, Jul 29, 2014 at 03:13:06PM +0200, Tomas Henzl wrote:
> Hi, 
> patch looks good.
> 
> A correction in pm8001_request_irq might be added too to allow 
> the INT-X path when pm8001_setup_msix fails, but that is not part 
> of this fix.


There's actually an equivalent fix already in through one of the
previous pm8001 updates, so I skipped this one even if it already had an
ACK.  The following pm8001 applied just fine, though.  Take a look, as
I've already pushed out the updated drivers branch.

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomas Henzl July 29, 2014, 1:48 p.m. UTC | #3 
On 07/29/2014 03:20 PM, Christoph Hellwig wrote:
> On Tue, Jul 29, 2014 at 03:13:06PM +0200, Tomas Henzl wrote:
>> Hi, 
>> patch looks good.
>>
>> A correction in pm8001_request_irq might be added too to allow 
>> the INT-X path when pm8001_setup_msix fails, but that is not part 
>> of this fix.
>
> There's actually an equivalent fix already in through one of the
> previous pm8001 updates, so I skipped this one even if it already had an
> ACK.  The following pm8001 applied just fine, though.  Take a look, as
> I've already pushed out the updated drivers branch.

I'm a bit confused - you were not commenting my text above I think?

I see in drivers-for-3.17 this patch already applied 5607de7
but not yet the next one (18/23) which also has a review, is there another ack needed?

>
> --
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig July 29, 2014, 1:56 p.m. UTC | #4 
On Tue, Jul 29, 2014 at 03:48:29PM +0200, Tomas Henzl wrote:
> I'm a bit confused - you were not commenting my text above I think?
> 
> I see in drivers-for-3.17 this patch already applied 5607de7
> but not yet the next one (18/23) which also has a review, is there another ack needed?

Looks like I got a bit confused due to a non-applying patch, but it
seems like the one that failed was the second one.  I'll look at it time
permitting, but if someone familar with pm8001 can fix the reject for me
that would help me greatly!

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Gordeev July 29, 2014, 2:15 p.m. UTC | #5 
On Tue, Jul 29, 2014 at 06:56:41AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 29, 2014 at 03:48:29PM +0200, Tomas Henzl wrote:
> > I'm a bit confused - you were not commenting my text above I think?
> > 
> > I see in drivers-for-3.17 this patch already applied 5607de7
> > but not yet the next one (18/23) which also has a review, is there another ack needed?
> 
> Looks like I got a bit confused due to a non-applying patch, but it
> seems like the one that failed was the second one.  I'll look at it time
> permitting, but if someone familar with pm8001 can fix the reject for me
> that would help me greatly!

Hmm.. 18/23 applies with a minor fuzz against
git://git.infradead.org/users/hch/scsi-queue.git drivers-for-3.17
Christoph Hellwig July 29, 2014, 2:24 p.m. UTC | #6 
On Tue, Jul 29, 2014 at 04:15:45PM +0200, Alexander Gordeev wrote:
> Hmm.. 18/23 applies with a minor fuzz against
> git://git.infradead.org/users/hch/scsi-queue.git drivers-for-3.17

Okay, it was just git-am being goofy then.  I'll take care of it.

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Gordeev Aug. 11, 2014, 8:14 a.m. UTC | #7 
On Tue, Jul 29, 2014 at 07:24:03AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 29, 2014 at 04:15:45PM +0200, Alexander Gordeev wrote:
> > Hmm.. 18/23 applies with a minor fuzz against
> > git://git.infradead.org/users/hch/scsi-queue.git drivers-for-3.17
> 
> Okay, it was just git-am being goofy then.  I'll take care of it.
> 

Anything I can help with?

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index e90c89f..e837ece 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -744,9 +744,10 @@  static u32 pm8001_setup_msix(struct pm8001_hba_info *pm8001_ha)
 			pm8001_ha->irq_vector[i].irq_id = i;
 			pm8001_ha->irq_vector[i].drv_inst = pm8001_ha;
 
-			if (request_irq(pm8001_ha->msix_entries[i].vector,
+			rc = request_irq(pm8001_ha->msix_entries[i].vector,
 				pm8001_interrupt_handler_msix, flag,
-				intr_drvname[i], &(pm8001_ha->irq_vector[i]))) {
+				intr_drvname[i], &(pm8001_ha->irq_vector[i]));
+			if (rc) {
 				for (j = 0; j < i; j++)
 					free_irq(
 					pm8001_ha->msix_entries[j].vector,