The Rockchip driver has since interrupt support was added used
request_threaded_irq() but not actually supplied a threaded handler,
handling everything in the primary handler.  This is equivalent to just
using a plain request_irq(), and since aef30c8d569c (genirq: Warn about
using IRQF_ONESHOT without a threaded handler) the current behaviour has
triggered a WARN_ON().  Convert to use request_irq().

Reported-by: Aishwarya TCV <Aishwarya.TCV@arm.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-rockchip.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 1a6381de6f33..62e1bc08c940 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -805,8 +805,8 @@ static int rockchip_spi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_put_ctlr;
 
-	ret = devm_request_threaded_irq(&pdev->dev, ret, rockchip_spi_isr, NULL,
-					IRQF_ONESHOT, dev_name(&pdev->dev), ctlr);
+	ret = devm_request_irq(&pdev->dev, ret, rockchip_spi_isr, 0,
+			       dev_name(&pdev->dev), ctlr);
 	if (ret)
 		goto err_put_ctlr;
 

---
base-commit: 0f61b1860cc3f52aef9036d7235ed1f017632193
change-id: 20260115-spi-rockchip-threaded-irq-b1641d0d3919

Best regards,
--  
Mark Brown <broonie@kernel.org>

On Fri, 16 Jan 2026 13:23:40 +0000, Mark Brown wrote:
> The Rockchip driver has since interrupt support was added used
> request_threaded_irq() but not actually supplied a threaded handler,
> handling everything in the primary handler.  This is equivalent to just
> using a plain request_irq(), and since aef30c8d569c (genirq: Warn about
> using IRQF_ONESHOT without a threaded handler) the current behaviour has
> triggered a WARN_ON().  Convert to use request_irq().
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: rockchip: Use plain request_irq()
      commit: 83ba6efa711fad83a0fbf02178ad64d3906d8d09

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

在 2026/01/16 星期五 21:23, Mark Brown 写道:
> The Rockchip driver has since interrupt support was added used
> request_threaded_irq() but not actually supplied a threaded handler,
> handling everything in the primary handler.  This is equivalent to just
> using a plain request_irq(), and since aef30c8d569c (genirq: Warn about
> using IRQF_ONESHOT without a threaded handler) the current behaviour has
> triggered a WARN_ON().  Convert to use request_irq().
> 

Is it preferred to use threaded version if latency is not a critical
concern ? I guess the original intention was to use

ret = devm_request_threaded_irq(&pdev->dev, ret, NULL, rockchip_spi_isr,
IRQF_ONESHOT, dev_name(&pdev->dev), ctlr); ?


> Reported-by: Aishwarya TCV <Aishwarya.TCV@arm.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   drivers/spi/spi-rockchip.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> index 1a6381de6f33..62e1bc08c940 100644
> --- a/drivers/spi/spi-rockchip.c
> +++ b/drivers/spi/spi-rockchip.c
> @@ -805,8 +805,8 @@ static int rockchip_spi_probe(struct platform_device *pdev)
>   	if (ret < 0)
>   		goto err_put_ctlr;
>   
> -	ret = devm_request_threaded_irq(&pdev->dev, ret, rockchip_spi_isr, NULL,
> -					IRQF_ONESHOT, dev_name(&pdev->dev), ctlr);
> +	ret = devm_request_irq(&pdev->dev, ret, rockchip_spi_isr, 0,
> +			       dev_name(&pdev->dev), ctlr);
>   	if (ret)
>   		goto err_put_ctlr;
>   
> 
> ---
> base-commit: 0f61b1860cc3f52aef9036d7235ed1f017632193
> change-id: 20260115-spi-rockchip-threaded-irq-b1641d0d3919
> 
> Best regards,
> --
> Mark Brown <broonie@kernel.org>
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 

On Fri, Jan 16, 2026 at 11:19:39PM +0800, Shawn Lin wrote:
> 在 2026/01/16 星期五 21:23, Mark Brown 写道:

> > The Rockchip driver has since interrupt support was added used
> > request_threaded_irq() but not actually supplied a threaded handler,
> > handling everything in the primary handler.  This is equivalent to just
> > using a plain request_irq(), and since aef30c8d569c (genirq: Warn about
> > using IRQF_ONESHOT without a threaded handler) the current behaviour has
> > triggered a WARN_ON().  Convert to use request_irq().

> Is it preferred to use threaded version if latency is not a critical
> concern ? I guess the original intention was to use

> ret = devm_request_threaded_irq(&pdev->dev, ret, NULL, rockchip_spi_isr,
> IRQF_ONESHOT, dev_name(&pdev->dev), ctlr); ?

TBH it looked to me more like there'd been some deferral of more complex
work at some point but that didn't make it into the final code.  In
general it's better to handle things in hardirq context if they are
appropriate for that, and for a SPI controller the end requirements are
coming from the device so you have to assume they're going to need
things to complete promptly.  There's overhead from scheduling a task so
no point in incurring it if it doesn't buy you anything.

+ Xuhui from rockchip who may keep an eye on spi-rockchip related topic

在 2026/01/16 星期五 23:37, Mark Brown 写道:
> On Fri, Jan 16, 2026 at 11:19:39PM +0800, Shawn Lin wrote:
>> 在 2026/01/16 星期五 21:23, Mark Brown 写道:
> 
>>> The Rockchip driver has since interrupt support was added used
>>> request_threaded_irq() but not actually supplied a threaded handler,
>>> handling everything in the primary handler.  This is equivalent to just
>>> using a plain request_irq(), and since aef30c8d569c (genirq: Warn about
>>> using IRQF_ONESHOT without a threaded handler) the current behaviour has
>>> triggered a WARN_ON().  Convert to use request_irq().
> 
>> Is it preferred to use threaded version if latency is not a critical
>> concern ? I guess the original intention was to use
> 
>> ret = devm_request_threaded_irq(&pdev->dev, ret, NULL, rockchip_spi_isr,
>> IRQF_ONESHOT, dev_name(&pdev->dev), ctlr); ?
> 
> TBH it looked to me more like there'd been some deferral of more complex
> work at some point but that didn't make it into the final code.  In

Ah, indeed，rockchip_spi_isr() directly performs R/W pio in hardirq
context which doesn't seem advisable, so probably some better
improvement is needed in the furture. Anyway, fix the warning right
now as $subject patch looks sensible:

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

- Thanks.

> general it's better to handle things in hardirq context if they are
> appropriate for that, and for a SPI controller the end requirements are
> coming from the device so you have to assume they're going to need
> things to complete promptly.  There's overhead from scheduling a task so
> no point in incurring it if it doesn't buy you anything.

On Sat, Jan 17, 2026 at 10:10:09AM +0800, Shawn Lin wrote:
> 在 2026/01/16 星期五 23:37, Mark Brown 写道:

> > TBH it looked to me more like there'd been some deferral of more complex
> > work at some point but that didn't make it into the final code.  In

> Ah, indeed，rockchip_spi_isr() directly performs R/W pio in hardirq
> context which doesn't seem advisable, so probably some better
> improvement is needed in the furture. Anyway, fix the warning right
> now as $subject patch looks sensible:

Generally the best pattern with this stuff is to do PIO in process
context - it looks like the driver already has a copybreak to use DMA
for larger transfers on systems that have DMA support so this should
only be happening for very short transfers in which case the context
thrashing from going to interrupt context is probably hurting
performance anyway.