| Message ID | f11593e75ba2e18e3b76989255cbb2e53a0213b4.1665034244.git.christophe.leroy@csgroup.eu |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: mac_scsi: Replace NO_IRQ by 0 | expand |
Le 06/10/2022 à 11:25, Finn Thain a écrit : > On Thu, 6 Oct 2022, Christophe Leroy wrote: > >> NO_IRQ is used to check the return of irq_of_parse_and_map(). >> >> On some architecture NO_IRQ is 0, on other architectures it is -1. >> > > Yes. The core NCR5380 driver is used on ARM etc. where NO_IRQ is -1 as > well as on powerpc where it is 0. > >> irq_of_parse_and_map() returns 0 on error, independent of NO_IRQ. >> >> So use 0 instead of using NO_IRQ. >> > > Sorry, I must be missing something. My mistake. I started by removing NO_IRQ definition in powerpc and then fixed all build failures by replacing NO_IRQ by 0. Then I splitted the patch into one per subsystem, all with the same explaination. Most places it was just a verification of the value returned by irq_of_parse_and_map() where it is obviously wrong to use NO_IRQ, especially on ARM where NO_IRQ doesn't match what irq_of_parse_and_map() returns in case on error. But here in the mac_scsi driver it seems a bit different and I have a look more closely. > > You seem to be saying that this driver could be re-used in the context of > openfirmware/device trees if it avoided using the NO_IRQ. Do I have that > right? > > Or are you changing NO_IRQ semantics tree-wide for some reason explained > somewhere else? No, I only say that NO_IRQ doesn't match the value returned by irq_of_parse_and_map(). Ultimately I want to remove the #define NO_IRQ from arch/powerpc/include/asm/irq.h That's to be linked to following message from Linus : https://lkml.org/lkml/2005/11/21/221 > > If it is the former, shouldn't you reverse the comment in > arch/powerpc/include/asm/irq.h, which says the macro is to be used in the > way this driver (and others) use it? > > If it is the latter, shouldn't you address the use of NO_IRQ in the core > NCR5380 driver rather than just this wrapper? Yes I guess so. > > Moreover, wouldn't it make more sense to fix the callers of > irq_of_parse_and_map(), since they appear to be abusing the NO_IRQ macro? Indeed. That's what is being done most places. > > For example, drivers/ata/sata_dwc_460ex.c actually does #define NO_IRQ 0 > and then expects irq_of_parse_and_map() will somehow use the same value to > mean the same thing... It didn't pop up during the multi-build I did for powerpc, so I guess that driver is not used for powerpc ? In the ata subsystem I fixed pata_mpc52xx. Thanks Christophe
diff --git a/drivers/scsi/mac_scsi.c b/drivers/scsi/mac_scsi.c index 2e511697fce3..e4df2b501e8b 100644 --- a/drivers/scsi/mac_scsi.c +++ b/drivers/scsi/mac_scsi.c @@ -478,7 +478,7 @@ static int __init mac_scsi_probe(struct platform_device *pdev) if (irq) instance->irq = irq->start; else - instance->irq = NO_IRQ; + instance->irq = 0; hostdata = shost_priv(instance); hostdata->base = pio_mem->start; @@ -495,7 +495,7 @@ static int __init mac_scsi_probe(struct platform_device *pdev) if (error) goto fail_init; - if (instance->irq != NO_IRQ) { + if (instance->irq) { error = request_irq(instance->irq, macscsi_intr, IRQF_SHARED, "NCR5380", instance); if (error) @@ -514,7 +514,7 @@ static int __init mac_scsi_probe(struct platform_device *pdev) return 0; fail_host: - if (instance->irq != NO_IRQ) + if (instance->irq) free_irq(instance->irq, instance); fail_irq: NCR5380_exit(instance); @@ -528,7 +528,7 @@ static int __exit mac_scsi_remove(struct platform_device *pdev) struct Scsi_Host *instance = platform_get_drvdata(pdev); scsi_remove_host(instance); - if (instance->irq != NO_IRQ) + if (instance->irq) free_irq(instance->irq, instance); NCR5380_exit(instance); scsi_host_put(instance);
NO_IRQ is used to check the return of irq_of_parse_and_map(). On some architecture NO_IRQ is 0, on other architectures it is -1. irq_of_parse_and_map() returns 0 on error, independent of NO_IRQ. So use 0 instead of using NO_IRQ. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- drivers/scsi/mac_scsi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)