[0/2] spi: spl022: fix sleeping in interrupt context

Hi,

While running the spl022, I got the following warning:
BUG: sleeping function called from invalid context at drivers/spi/spi.c:1428

This is because between spi transfers, spi_transfer_delay_exec() (who
may sleep if the delay is >10us) is called in interrupt context. This is
a problem for anyone who runs this driver and need more than 10us delay.

Patch 1 adds an error reporting mechanism, needed by patch 2 who switch
to use the default spi_transfer_one_message(), which fix the problem.

The series is tested with polling transfer mode and interrupt transfer
mode. I can't test the DMA mode, so some help testing here is very
appreciated.

One question: This series is quite big for stable trees, so how can we
backport this fix? We can:
  - Let it be released, and get tested for some time. After a while
    without any reported problem, backport it.
  - Have a small patch which fixes this problem. One idea I have is to
    switch the current interrupt handler to threaded interrupt handler,
    and switch from existing use of tasklet to workqueue. So that the
    driver can safely sleep if needed. And then add this series on top
    of that.
  - other options that I miss?

Best regards,
Nam

Nam Cao (2):
  spi: introduce SPI_TRANS_FAIL_IO for error reporting
  spi: spl022: switch to use default spi_transfer_one_message()

 drivers/spi/spi-pl022.c | 372 +++++++---------------------------------
 drivers/spi/spi.c       |   3 +
 include/linux/spi/spi.h |   1 +
 3 files changed, 70 insertions(+), 306 deletions(-)

On Wed, Nov 29, 2023 at 2:32 PM Nam Cao <namcao@linutronix.de> wrote:

> The default message transfer implementation - spi_transfer_one_message -
> invokes the specific device driver's transfer_one(), then waits for
> completion. However, there is no mechanism for the device driver to
> report failure in the middle of the transfer.
>
> Introduce SPI_TRANS_FAIL_IO for drivers to report transfer failure.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>

This looks useful to me
Acked-by: Linus Walleij <linus.walleij@linaro.org>

>  #define SPI_TRANS_FAIL_NO_START        BIT(0)
> +#define SPI_TRANS_FAIL_IO      BIT(1)

Is it obvious from context when these flags get set?
from transfer_one() and in which flag field?

Otherwise maybe we should add a comment.

Yours,
Linus Walleij

On Wed, Nov 29, 2023 at 02:55:18PM +0100, Linus Walleij wrote:
> On Wed, Nov 29, 2023 at 2:32 PM Nam Cao <namcao@linutronix.de> wrote:
> >  #define SPI_TRANS_FAIL_NO_START        BIT(0)
> > +#define SPI_TRANS_FAIL_IO      BIT(1)
> 
> Is it obvious from context when these flags get set?
> from transfer_one() and in which flag field?
> 
> Otherwise maybe we should add a comment.

Agree that the purpose of this flag is not clear. I will add some
description in v2.

Best regards,
Nam