On Thu, 26 Nov 2020, Sebastian Andrzej Siewior wrote:
> From: "Ahmed S. Darwish" <a.darwish@xxxxxxxxxxxxx>
>
> NCR5380_poll_politely2() uses in_interrupt() to check if it is safe to
> sleep.
>
> The usage of in_interrupt() 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.
>
> If direct callers are purely atomic, or purely task context, change
> their specifications accordingly and mark them with "Context: " tags.
>
> For the mixed ones, trickle-down context from upper layers.
>
> Signed-off-by: Ahmed S. Darwish <a.darwish@xxxxxxxxxxxxx>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
Acked-by: Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx>
> @@ -513,9 +513,11 @@ static void wait_for_53c80_access(struct NCR5380_hostdata *hostdata)
> * @dst: buffer to write into
> * @len: transfer size
> *
> + * Context: atomic. This implements NCR5380.c NCR5380_dma_recv_setup(),
> + * which is always called with @hostdata spinlock held.
> + *
> * Perform a pseudo DMA mode receive from a 53C400 or equivalent device.
> */
> -
> static inline int generic_NCR5380_precv(struct NCR5380_hostdata *hostdata,
> unsigned char *dst, int len)
> {
BTW, if I was doing this, I'd omit all of the many gratuitous whitespace
changes and I'd avoid copying so much program logic into the comments
where it is redundant -- here I'd have written only "Context: atomic,
spinlock held". However, no need to revise. Looks fine otherwise.
