| Message ID | 810f26d3-908b-1d6b-dc5c-40019726baca@omprussia.ru |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: hisi_sas: fix IRQ checks | expand |
On 4/4/21 2:48 PM, Andy Shevchenko wrote: [...] > Commit df2d8213d9e3 ("hisi_sas: use platform_get_irq()") failed to take > into account that irq_of_parse_and_map() and platform_get_irq() have a > different way of indicating an error: the former returns 0 and the latter > returns a negative error code. Fix up the IRQ checks! > > > Shouldn’t you unshadow error codes at the same time? > > return -ENOENT; ==> return IRQ; I'm going to send that as a follow-up (cleanup) patch -- we also have devm_request_irq() with the result overridden for no good reason... > Fixes: df2d8213d9e3 ("hisi_sas: use platform_get_irq()") > Signed-off-by: Sergey Shtylyov <s.shtylyov@omprussia.ru <mailto:s.shtylyov@omprussia.ru>> [...] MBR, Sergei
On 4/3/21 11:43 PM, Sergey Shtylyov wrote: > Commit df2d8213d9e3 ("hisi_sas: use platform_get_irq()") failed to take > into account that irq_of_parse_and_map() and platform_get_irq() have a > different way of indicating an error: the former returns 0 and the latter > returns a negative error code. Fix up the IRQ checks! > > Fixes: df2d8213d9e3 ("hisi_sas: use platform_get_irq()") > Signed-off-by: Sergey Shtylyov <s.shtylyov@omprussia.ru> > > --- Sorry, forgot to mention that this patch is against the 'fixes' branch of Martin Petgersen's 'scsi.git' repo. [...] MBR, Sergey
On 05/04/2021 15:27, Sergey Shtylyov wrote: > On 4/3/21 11:43 PM, Sergey Shtylyov wrote: > >> Commit df2d8213d9e3 ("hisi_sas: use platform_get_irq()") failed to take >> into account that irq_of_parse_and_map() and platform_get_irq() have a >> different way of indicating an error: the former returns 0 and the latter >> returns a negative error code. Fix up the IRQ checks! >> >> Fixes: df2d8213d9e3 ("hisi_sas: use platform_get_irq()") >> Signed-off-by: Sergey Shtylyov <s.shtylyov@omprussia.ru> >> >> --- > > Sorry, forgot to mention that this patch is against the 'fixes' branch of > Martin Petgersen's 'scsi.git' repo. JFYI, The HW for this v1 hw driver is all but dead, and I was considering deleting the driver. But, for now, if you want to fix up to ensure no one copies this pattern, then fine. Thanks, John
Hello! On 4/6/21 10:43 AM, John Garry wrote: [...] >>> Commit df2d8213d9e3 ("hisi_sas: use platform_get_irq()") failed to take >>> into account that irq_of_parse_and_map() and platform_get_irq() have a >>> different way of indicating an error: the former returns 0 and the latter >>> returns a negative error code. Fix up the IRQ checks! >>> >>> Fixes: df2d8213d9e3 ("hisi_sas: use platform_get_irq()") >>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omprussia.ru> >>> >>> --- >> >> Sorry, forgot to mention that this patch is against the 'fixes' branch of >> Martin Petgersen's 'scsi.git' repo. > > JFYI, The HW for this v1 hw driver is all but dead, and I was considering deleting the driver. > > But, for now, if you want to fix up to ensure no one copies this pattern, then fine. Yeah, that too. And the -stable kernels also need to be considered... > Thanks, > John MBR, Sergey
On Sat, 3 Apr 2021 23:43:55 +0300, Sergey Shtylyov wrote: > Commit df2d8213d9e3 ("hisi_sas: use platform_get_irq()") failed to take > into account that irq_of_parse_and_map() and platform_get_irq() have a > different way of indicating an error: the former returns 0 and the latter > returns a negative error code. Fix up the IRQ checks! Applied to 5.13/scsi-queue, thanks! [1/1] scsi: hisi_sas: fix IRQ checks https://git.kernel.org/mkp/scsi/c/6c11dc060427 -- Martin K. Petersen Oracle Linux Engineering
Index: scsi/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c =================================================================== --- scsi.orig/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c +++ scsi/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c @@ -1646,7 +1646,7 @@ static int interrupt_init_v1_hw(struct h idx = i * HISI_SAS_PHY_INT_NR; for (j = 0; j < HISI_SAS_PHY_INT_NR; j++, idx++) { irq = platform_get_irq(pdev, idx); - if (!irq) { + if (irq < 0) { dev_err(dev, "irq init: fail map phy interrupt %d\n", idx); return -ENOENTr; @@ -1665,7 +1665,7 @@ static int interrupt_init_v1_hw(struct h idx = hisi_hba->n_phy * HISI_SAS_PHY_INT_NR; for (i = 0; i < hisi_hba->queue_count; i++, idx++) { irq = platform_get_irq(pdev, idx); - if (!irq) { + if (irq < 0) { dev_err(dev, "irq init: could not map cq interrupt %d\n", idx); return -ENOENT; @@ -1683,7 +1683,7 @@ static int interrupt_init_v1_hw(struct h idx = (hisi_hba->n_phy * HISI_SAS_PHY_INT_NR) + hisi_hba->queue_count; for (i = 0; i < HISI_SAS_FATAL_INT_NR; i++, idx++) { irq = platform_get_irq(pdev, idx); - if (!irq) { + if (irq < 0) { dev_err(dev, "irq init: could not map fatal interrupt %d\n", idx); return -ENOENT;
Commit df2d8213d9e3 ("hisi_sas: use platform_get_irq()") failed to take into account that irq_of_parse_and_map() and platform_get_irq() have a different way of indicating an error: the former returns 0 and the latter returns a negative error code. Fix up the IRQ checks! Fixes: df2d8213d9e3 ("hisi_sas: use platform_get_irq()") Signed-off-by: Sergey Shtylyov <s.shtylyov@omprussia.ru> --- drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)