Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

scsi: NCR5380: Remove context check

NCR5380_poll_politely2() uses in_interrupt() and irqs_disabled() to check
if it is safe to sleep.

Such usage in drivers is phased out and Linus clearly requested that code
which changes behaviour depending on context should either be separated, or
the context be explicitly conveyed in an argument passed by the caller.

Below is a context analysis of NCR5380_poll_politely2() uppermost callers:

- NCR5380_maybe_reset_bus(), task, invoked during device probe.
-> NCR5380_poll_politely()
-> do_abort()

- NCR5380_select(), task, but can only sleep in the "release, then
re-acquire" regions of the spinlock held by its caller.
Sleeping invocations (lock released):
-> NCR5380_poll_politely2()

Atomic invocations (lock acquired):
-> NCR5380_reselect()
-> NCR5380_poll_politely()
-> do_abort()
-> NCR5380_transfer_pio()

- NCR5380_intr(), interrupt handler
-> NCR5380_dma_complete()
-> NCR5380_transfer_pio()
-> NCR5380_poll_politely()
-> NCR5380_reselect() (see above)

- NCR5380_information_transfer(), task, but can only sleep in the
"release, then re-acquire" regions of the caller-held spinlock.
Sleeping invocations (lock released):
- NCR5380_transfer_pio() -> NCR5380_poll_politely()
- NCR5380_poll_politely()

Atomic invocations (lock acquired):
- NCR5380_transfer_dma()
-> NCR5380_dma_recv_setup()
=> generic_NCR5380_precv() -> NCR5380_poll_politely()
=> macscsi_pread() -> NCR5380_poll_politely()

-> NCR5380_dma_send_setup()
=> generic_NCR5380_psend -> NCR5380_poll_politely2()
=> macscsi_pwrite() -> NCR5380_poll_politely()

-> NCR5380_poll_politely2()
-> NCR5380_dma_complete()
-> NCR5380_transfer_pio()
-> NCR5380_poll_politely()
- NCR5380_transfer_pio() -> NCR5380_poll_politely

- NCR5380_reselect(), atomic, always called with hostdata spinlock
held.

Since NCR5380_poll_politely2() already takes a "wait" argument in jiffies,
use it to determine if the function can sleep. Modify atomic callers, which
passed an unused wait value in terms of HZ, to pass zero.

Link: https://lore.kernel.org/r/20201206075157.19067-1-a.darwish@linutronix.de
Cc: Michael Schmitz <schmitzmic@gmail.com>
Cc: <linux-m68k@lists.linux-m68k.org>
Suggested-by: Finn Thain <fthain@telegraphics.com.au>
Co-developed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

authored by

Ahmed S. Darwish and committed by
Martin K. Petersen
e7734ef1 8ca1a40b

+53 -46
+40 -34
drivers/scsi/NCR5380.c
··· 132 132 static unsigned int disconnect_mask = ~0; 133 133 module_param(disconnect_mask, int, 0444); 134 134 135 - static int do_abort(struct Scsi_Host *); 135 + static int do_abort(struct Scsi_Host *, unsigned int); 136 136 static void do_reset(struct Scsi_Host *); 137 137 static void bus_reset_cleanup(struct Scsi_Host *); 138 138 ··· 197 197 * @reg2: Second 5380 register to poll 198 198 * @bit2: Second bitmask to check 199 199 * @val2: Second expected value 200 - * @wait: Time-out in jiffies 200 + * @wait: Time-out in jiffies, 0 if sleeping is not allowed 201 201 * 202 202 * Polls the chip in a reasonably efficient manner waiting for an 203 203 * event to occur. After a short quick poll we begin to yield the CPU ··· 223 223 cpu_relax(); 224 224 } while (n--); 225 225 226 - if (irqs_disabled() || in_interrupt()) 226 + if (!wait) 227 227 return -ETIMEDOUT; 228 228 229 229 /* Repeatedly sleep for 1 ms until deadline */ ··· 486 486 break; 487 487 case 2: 488 488 shost_printk(KERN_ERR, instance, "bus busy, attempting abort\n"); 489 - do_abort(instance); 489 + do_abort(instance, 1); 490 490 break; 491 491 case 4: 492 492 shost_printk(KERN_ERR, instance, "bus busy, attempting reset\n"); ··· 822 822 if (toPIO > 0) { 823 823 dsprintk(NDEBUG_DMA, instance, 824 824 "Doing %d byte PIO to 0x%p\n", cnt, *data); 825 - NCR5380_transfer_pio(instance, &p, &cnt, data); 825 + NCR5380_transfer_pio(instance, &p, &cnt, data, 0); 826 826 *count -= toPIO - cnt; 827 827 } 828 828 } ··· 1189 1189 goto out; 1190 1190 } 1191 1191 if (!hostdata->selecting) { 1192 - do_abort(instance); 1192 + do_abort(instance, 0); 1193 1193 return false; 1194 1194 } 1195 1195 ··· 1200 1200 len = 1; 1201 1201 data = tmp; 1202 1202 phase = PHASE_MSGOUT; 1203 - NCR5380_transfer_pio(instance, &phase, &len, &data); 1203 + NCR5380_transfer_pio(instance, &phase, &len, &data, 0); 1204 1204 if (len) { 1205 1205 NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE); 1206 1206 cmd->result = DID_ERROR << 16; ··· 1238 1238 * 1239 1239 * Inputs : instance - instance of driver, *phase - pointer to 1240 1240 * what phase is expected, *count - pointer to number of 1241 - * bytes to transfer, **data - pointer to data pointer. 1241 + * bytes to transfer, **data - pointer to data pointer, 1242 + * can_sleep - 1 or 0 when sleeping is permitted or not, respectively. 1242 1243 * 1243 1244 * Returns : -1 when different phase is entered without transferring 1244 1245 * maximum number of bytes, 0 if all bytes are transferred or exit ··· 1258 1257 1259 1258 static int NCR5380_transfer_pio(struct Scsi_Host *instance, 1260 1259 unsigned char *phase, int *count, 1261 - unsigned char **data) 1260 + unsigned char **data, unsigned int can_sleep) 1262 1261 { 1263 1262 struct NCR5380_hostdata *hostdata = shost_priv(instance); 1264 1263 unsigned char p = *phase, tmp; ··· 1279 1278 * valid 1280 1279 */ 1281 1280 1282 - if (NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, HZ) < 0) 1281 + if (NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, 1282 + HZ * can_sleep) < 0) 1283 1283 break; 1284 1284 1285 1285 dsprintk(NDEBUG_HANDSHAKE, instance, "REQ asserted\n"); ··· 1326 1324 } 1327 1325 1328 1326 if (NCR5380_poll_politely(hostdata, 1329 - STATUS_REG, SR_REQ, 0, 5 * HZ) < 0) 1327 + STATUS_REG, SR_REQ, 0, 5 * HZ * can_sleep) < 0) 1330 1328 break; 1331 1329 1332 1330 dsprintk(NDEBUG_HANDSHAKE, instance, "REQ negated, handshake complete\n"); ··· 1401 1399 * do_abort - abort the currently established nexus by going to 1402 1400 * MESSAGE OUT phase and sending an ABORT message. 1403 1401 * @instance: relevant scsi host instance 1402 + * @can_sleep: 1 or 0 when sleeping is permitted or not, respectively 1404 1403 * 1405 1404 * Returns 0 on success, negative error code on failure. 1406 1405 */ 1407 1406 1408 - static int do_abort(struct Scsi_Host *instance) 1407 + static int do_abort(struct Scsi_Host *instance, unsigned int can_sleep) 1409 1408 { 1410 1409 struct NCR5380_hostdata *hostdata = shost_priv(instance); 1411 1410 unsigned char *msgptr, phase, tmp; ··· 1426 1423 * the target sees, so we just handshake. 1427 1424 */ 1428 1425 1429 - rc = NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, 10 * HZ); 1426 + rc = NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, 1427 + 10 * HZ * can_sleep); 1430 1428 if (rc < 0) 1431 1429 goto out; 1432 1430 ··· 1438 1434 if (tmp != PHASE_MSGOUT) { 1439 1435 NCR5380_write(INITIATOR_COMMAND_REG, 1440 1436 ICR_BASE | ICR_ASSERT_ATN | ICR_ASSERT_ACK); 1441 - rc = NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, 0, 3 * HZ); 1437 + rc = NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, 0, 1438 + 3 * HZ * can_sleep); 1442 1439 if (rc < 0) 1443 1440 goto out; 1444 1441 NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE | ICR_ASSERT_ATN); ··· 1449 1444 msgptr = &tmp; 1450 1445 len = 1; 1451 1446 phase = PHASE_MSGOUT; 1452 - NCR5380_transfer_pio(instance, &phase, &len, &msgptr); 1447 + NCR5380_transfer_pio(instance, &phase, &len, &msgptr, can_sleep); 1453 1448 if (len) 1454 1449 rc = -ENXIO; 1455 1450 ··· 1628 1623 */ 1629 1624 1630 1625 if (NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, 1631 - BASR_DRQ, BASR_DRQ, HZ) < 0) { 1626 + BASR_DRQ, BASR_DRQ, 0) < 0) { 1632 1627 result = -1; 1633 1628 shost_printk(KERN_ERR, instance, "PDMA read: DRQ timeout\n"); 1634 1629 } 1635 1630 if (NCR5380_poll_politely(hostdata, STATUS_REG, 1636 - SR_REQ, 0, HZ) < 0) { 1631 + SR_REQ, 0, 0) < 0) { 1637 1632 result = -1; 1638 1633 shost_printk(KERN_ERR, instance, "PDMA read: !REQ timeout\n"); 1639 1634 } ··· 1645 1640 */ 1646 1641 if (NCR5380_poll_politely2(hostdata, 1647 1642 BUS_AND_STATUS_REG, BASR_DRQ, BASR_DRQ, 1648 - BUS_AND_STATUS_REG, BASR_PHASE_MATCH, 0, HZ) < 0) { 1643 + BUS_AND_STATUS_REG, BASR_PHASE_MATCH, 0, 0) < 0) { 1649 1644 result = -1; 1650 1645 shost_printk(KERN_ERR, instance, "PDMA write: DRQ and phase timeout\n"); 1651 1646 } ··· 1742 1737 #if (NDEBUG & NDEBUG_NO_DATAOUT) 1743 1738 shost_printk(KERN_DEBUG, instance, "NDEBUG_NO_DATAOUT set, attempted DATAOUT aborted\n"); 1744 1739 sink = 1; 1745 - do_abort(instance); 1740 + do_abort(instance, 0); 1746 1741 cmd->result = DID_ERROR << 16; 1747 1742 complete_cmd(instance, cmd); 1748 1743 hostdata->connected = NULL; ··· 1798 1793 NCR5380_PIO_CHUNK_SIZE); 1799 1794 len = transfersize; 1800 1795 NCR5380_transfer_pio(instance, &phase, &len, 1801 - (unsigned char **)&cmd->SCp.ptr); 1796 + (unsigned char **)&cmd->SCp.ptr, 1797 + 0); 1802 1798 cmd->SCp.this_residual -= transfersize - len; 1803 1799 } 1804 1800 #ifdef CONFIG_SUN3 ··· 1810 1804 case PHASE_MSGIN: 1811 1805 len = 1; 1812 1806 data = &tmp; 1813 - NCR5380_transfer_pio(instance, &phase, &len, &data); 1807 + NCR5380_transfer_pio(instance, &phase, &len, &data, 0); 1814 1808 cmd->SCp.Message = tmp; 1815 1809 1816 1810 switch (tmp) { ··· 1916 1910 len = 2; 1917 1911 data = extended_msg + 1; 1918 1912 phase = PHASE_MSGIN; 1919 - NCR5380_transfer_pio(instance, &phase, &len, &data); 1913 + NCR5380_transfer_pio(instance, &phase, &len, &data, 1); 1920 1914 dsprintk(NDEBUG_EXTENDED, instance, "length %d, code 0x%02x\n", 1921 1915 (int)extended_msg[1], 1922 1916 (int)extended_msg[2]); ··· 1929 1923 data = extended_msg + 3; 1930 1924 phase = PHASE_MSGIN; 1931 1925 1932 - NCR5380_transfer_pio(instance, &phase, &len, &data); 1926 + NCR5380_transfer_pio(instance, &phase, &len, &data, 1); 1933 1927 dsprintk(NDEBUG_EXTENDED, instance, "message received, residual %d\n", 1934 1928 len); 1935 1929 ··· 1976 1970 len = 1; 1977 1971 data = &msgout; 1978 1972 hostdata->last_message = msgout; 1979 - NCR5380_transfer_pio(instance, &phase, &len, &data); 1973 + NCR5380_transfer_pio(instance, &phase, &len, &data, 0); 1980 1974 if (msgout == ABORT) { 1981 1975 hostdata->connected = NULL; 1982 1976 hostdata->busy[scmd_id(cmd)] &= ~(1 << cmd->device->lun); ··· 1994 1988 * PSEUDO-DMA architecture we should probably 1995 1989 * use the dma transfer function. 1996 1990 */ 1997 - NCR5380_transfer_pio(instance, &phase, &len, &data); 1991 + NCR5380_transfer_pio(instance, &phase, &len, &data, 0); 1998 1992 break; 1999 1993 case PHASE_STATIN: 2000 1994 len = 1; 2001 1995 data = &tmp; 2002 - NCR5380_transfer_pio(instance, &phase, &len, &data); 1996 + NCR5380_transfer_pio(instance, &phase, &len, &data, 0); 2003 1997 cmd->SCp.Status = tmp; 2004 1998 break; 2005 1999 default: ··· 2058 2052 2059 2053 NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE | ICR_ASSERT_BSY); 2060 2054 if (NCR5380_poll_politely(hostdata, 2061 - STATUS_REG, SR_SEL, 0, 2 * HZ) < 0) { 2055 + STATUS_REG, SR_SEL, 0, 0) < 0) { 2062 2056 shost_printk(KERN_ERR, instance, "reselect: !SEL timeout\n"); 2063 2057 NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE); 2064 2058 return; ··· 2070 2064 */ 2071 2065 2072 2066 if (NCR5380_poll_politely(hostdata, 2073 - STATUS_REG, SR_REQ, SR_REQ, 2 * HZ) < 0) { 2067 + STATUS_REG, SR_REQ, SR_REQ, 0) < 0) { 2074 2068 if ((NCR5380_read(STATUS_REG) & (SR_BSY | SR_SEL)) == 0) 2075 2069 /* BUS FREE phase */ 2076 2070 return; 2077 2071 shost_printk(KERN_ERR, instance, "reselect: REQ timeout\n"); 2078 - do_abort(instance); 2072 + do_abort(instance, 0); 2079 2073 return; 2080 2074 } 2081 2075 ··· 2091 2085 unsigned char *data = msg; 2092 2086 unsigned char phase = PHASE_MSGIN; 2093 2087 2094 - NCR5380_transfer_pio(instance, &phase, &len, &data); 2088 + NCR5380_transfer_pio(instance, &phase, &len, &data, 0); 2095 2089 2096 2090 if (len) { 2097 - do_abort(instance); 2091 + do_abort(instance, 0); 2098 2092 return; 2099 2093 } 2100 2094 } ··· 2104 2098 shost_printk(KERN_ERR, instance, "expecting IDENTIFY message, got "); 2105 2099 spi_print_msg(msg); 2106 2100 printk("\n"); 2107 - do_abort(instance); 2101 + do_abort(instance, 0); 2108 2102 return; 2109 2103 } 2110 2104 lun = msg[0] & 0x07; ··· 2144 2138 * Since we have an established nexus that we can't do anything 2145 2139 * with, we must abort it. 2146 2140 */ 2147 - if (do_abort(instance) == 0) 2141 + if (do_abort(instance, 0) == 0) 2148 2142 hostdata->busy[target] &= ~(1 << lun); 2149 2143 return; 2150 2144 } ··· 2291 2285 dsprintk(NDEBUG_ABORT, instance, "abort: cmd %p is connected\n", cmd); 2292 2286 hostdata->connected = NULL; 2293 2287 hostdata->dma_len = 0; 2294 - if (do_abort(instance) < 0) { 2288 + if (do_abort(instance, 0) < 0) { 2295 2289 set_host_byte(cmd, DID_ERROR); 2296 2290 complete_cmd(instance, cmd); 2297 2291 result = FAILED;
+2 -1
drivers/scsi/NCR5380.h
··· 277 277 static void NCR5380_reselect(struct Scsi_Host *instance); 278 278 static bool NCR5380_select(struct Scsi_Host *, struct scsi_cmnd *); 279 279 static int NCR5380_transfer_dma(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data); 280 - static int NCR5380_transfer_pio(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data); 280 + static int NCR5380_transfer_pio(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data, 281 + unsigned int can_sleep); 281 282 static int NCR5380_poll_politely2(struct NCR5380_hostdata *, 282 283 unsigned int, u8, u8, 283 284 unsigned int, u8, u8, unsigned long);
+6 -6
drivers/scsi/g_NCR5380.c
··· 529 529 if (start == len - 128) { 530 530 /* Ignore End of DMA interrupt for the final buffer */ 531 531 if (NCR5380_poll_politely(hostdata, hostdata->c400_ctl_status, 532 - CSR_HOST_BUF_NOT_RDY, 0, HZ / 64) < 0) 532 + CSR_HOST_BUF_NOT_RDY, 0, 0) < 0) 533 533 break; 534 534 } else { 535 535 if (NCR5380_poll_politely2(hostdata, hostdata->c400_ctl_status, 536 536 CSR_HOST_BUF_NOT_RDY, 0, 537 537 hostdata->c400_ctl_status, 538 538 CSR_GATED_53C80_IRQ, 539 - CSR_GATED_53C80_IRQ, HZ / 64) < 0 || 539 + CSR_GATED_53C80_IRQ, 0) < 0 || 540 540 NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY) 541 541 break; 542 542 } ··· 565 565 if (residual == 0 && NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, 566 566 BASR_END_DMA_TRANSFER, 567 567 BASR_END_DMA_TRANSFER, 568 - HZ / 64) < 0) 568 + 0) < 0) 569 569 scmd_printk(KERN_ERR, hostdata->connected, "%s: End of DMA timeout\n", 570 570 __func__); 571 571 ··· 597 597 CSR_HOST_BUF_NOT_RDY, 0, 598 598 hostdata->c400_ctl_status, 599 599 CSR_GATED_53C80_IRQ, 600 - CSR_GATED_53C80_IRQ, HZ / 64) < 0 || 600 + CSR_GATED_53C80_IRQ, 0) < 0 || 601 601 NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY) { 602 602 /* Both 128 B buffers are in use */ 603 603 if (start >= 128) ··· 644 644 if (residual == 0) { 645 645 if (NCR5380_poll_politely(hostdata, TARGET_COMMAND_REG, 646 646 TCR_LAST_BYTE_SENT, TCR_LAST_BYTE_SENT, 647 - HZ / 64) < 0) 647 + 0) < 0) 648 648 scmd_printk(KERN_ERR, hostdata->connected, 649 649 "%s: Last Byte Sent timeout\n", __func__); 650 650 651 651 if (NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, 652 652 BASR_END_DMA_TRANSFER, BASR_END_DMA_TRANSFER, 653 - HZ / 64) < 0) 653 + 0) < 0) 654 654 scmd_printk(KERN_ERR, hostdata->connected, "%s: End of DMA timeout\n", 655 655 __func__); 656 656 }
+5 -5
drivers/scsi/mac_scsi.c
··· 285 285 286 286 while (!NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, 287 287 BASR_DRQ | BASR_PHASE_MATCH, 288 - BASR_DRQ | BASR_PHASE_MATCH, HZ / 64)) { 288 + BASR_DRQ | BASR_PHASE_MATCH, 0)) { 289 289 int bytes; 290 290 291 291 if (macintosh_config->ident == MAC_MODEL_IIFX) ··· 304 304 305 305 if (NCR5380_poll_politely2(hostdata, STATUS_REG, SR_REQ, SR_REQ, 306 306 BUS_AND_STATUS_REG, BASR_ACK, 307 - BASR_ACK, HZ / 64) < 0) 307 + BASR_ACK, 0) < 0) 308 308 scmd_printk(KERN_DEBUG, hostdata->connected, 309 309 "%s: !REQ and !ACK\n", __func__); 310 310 if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH)) ··· 344 344 345 345 while (!NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, 346 346 BASR_DRQ | BASR_PHASE_MATCH, 347 - BASR_DRQ | BASR_PHASE_MATCH, HZ / 64)) { 347 + BASR_DRQ | BASR_PHASE_MATCH, 0)) { 348 348 int bytes; 349 349 350 350 if (macintosh_config->ident == MAC_MODEL_IIFX) ··· 362 362 if (NCR5380_poll_politely(hostdata, TARGET_COMMAND_REG, 363 363 TCR_LAST_BYTE_SENT, 364 364 TCR_LAST_BYTE_SENT, 365 - HZ / 64) < 0) { 365 + 0) < 0) { 366 366 scmd_printk(KERN_ERR, hostdata->connected, 367 367 "%s: Last Byte Sent timeout\n", __func__); 368 368 result = -1; ··· 372 372 373 373 if (NCR5380_poll_politely2(hostdata, STATUS_REG, SR_REQ, SR_REQ, 374 374 BUS_AND_STATUS_REG, BASR_ACK, 375 - BASR_ACK, HZ / 64) < 0) 375 + BASR_ACK, 0) < 0) 376 376 scmd_printk(KERN_DEBUG, hostdata->connected, 377 377 "%s: !REQ and !ACK\n", __func__); 378 378 if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH))