public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] firmware: tegra: bpmp: Make atomic_tx_lock a raw_spinlock
@ 2026-05-04 16:34 Waiman Long
  2026-05-05  6:27 ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 3+ messages in thread
From: Waiman Long @ 2026-05-04 16:34 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Sebastian Andrzej Siewior,
	Clark Williams, Steven Rostedt
  Cc: linux-tegra, linux-kernel, linux-rt-devel, jberring, Waiman Long

The atomic_tx_lock was first introduced by commit 1abb081e41a7
("firmware: tegra: Simplify channel management") as a spinlock_t. It
is used only in tegra_bpmp_transfer_atomic() to ensure mutual exclusion.

Since the merging of PREEMPT_RT support into mainline Linux kernel
in v6.12, a spinlock becomes a sleeping lock when CONFIG_PREEMPT_RT
is enabled. As tegra_bpmp_transfer_atomic() is called with interrupt
disabled, acquiring a sleeping lock will lead to the following bug
report when booting up a PREEMPT_RT kernel on an tegra based arm64
system with Boot and Power Management Processor (BPMP).

  BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0
  preempt_count: 0, expected: 0
  RCU nest depth: 0, expected: 0
  2 locks held by swapper/0/1:
   #0: ffff42bda2d5b0f0 (&dev->mutex){....}-{3:3}, at: __driver_attach+0x110/0x2c0
   #1: ffff42bda4da5138 (&bpmp->atomic_tx_lock){....}-{2:2}, at: tegra_bpmp_transfer_atomic+0x118/0x3c0

Fix it by changing the type of atomic_tx_lock in the tegra_bpmp structure
to raw_spinlock_t and use raw_spinlock APIs to access it.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 drivers/firmware/tegra/bpmp.c | 8 ++++----
 include/soc/tegra/bpmp.h      | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
index 753472b53bd8..1a82ce7d340d 100644
--- a/drivers/firmware/tegra/bpmp.c
+++ b/drivers/firmware/tegra/bpmp.c
@@ -365,16 +365,16 @@ int tegra_bpmp_transfer_atomic(struct tegra_bpmp *bpmp,
 
 	channel = bpmp->tx_channel;
 
-	spin_lock(&bpmp->atomic_tx_lock);
+	raw_spin_lock(&bpmp->atomic_tx_lock);
 
 	err = tegra_bpmp_channel_write(channel, msg->mrq, MSG_ACK,
 				       msg->tx.data, msg->tx.size);
 	if (err < 0) {
-		spin_unlock(&bpmp->atomic_tx_lock);
+		raw_spin_unlock(&bpmp->atomic_tx_lock);
 		return err;
 	}
 
-	spin_unlock(&bpmp->atomic_tx_lock);
+	raw_spin_unlock(&bpmp->atomic_tx_lock);
 
 	err = tegra_bpmp_ring_doorbell(bpmp);
 	if (err < 0)
@@ -763,7 +763,7 @@ static int tegra_bpmp_probe(struct platform_device *pdev)
 	if (!bpmp->threaded.busy)
 		return -ENOMEM;
 
-	spin_lock_init(&bpmp->atomic_tx_lock);
+	raw_spin_lock_init(&bpmp->atomic_tx_lock);
 	bpmp->tx_channel = devm_kzalloc(&pdev->dev, sizeof(*bpmp->tx_channel),
 					GFP_KERNEL);
 	if (!bpmp->tx_channel)
diff --git a/include/soc/tegra/bpmp.h b/include/soc/tegra/bpmp.h
index a33582590a3b..fdd8739715dc 100644
--- a/include/soc/tegra/bpmp.h
+++ b/include/soc/tegra/bpmp.h
@@ -79,7 +79,7 @@ struct tegra_bpmp {
 		struct mbox_chan *channel;
 	} mbox;
 
-	spinlock_t atomic_tx_lock;
+	raw_spinlock_t atomic_tx_lock;
 	struct tegra_bpmp_channel *tx_channel, *rx_channel, *threaded_channels;
 
 	struct {
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] firmware: tegra: bpmp: Make atomic_tx_lock a raw_spinlock
  2026-05-04 16:34 [PATCH] firmware: tegra: bpmp: Make atomic_tx_lock a raw_spinlock Waiman Long
@ 2026-05-05  6:27 ` Sebastian Andrzej Siewior
       [not found]   ` <afdb5fc1-f478-4547-aa39-04d477854d64@redhat.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-05-05  6:27 UTC (permalink / raw)
  To: Waiman Long
  Cc: Thierry Reding, Jonathan Hunter, Clark Williams, Steven Rostedt,
	linux-tegra, linux-kernel, linux-rt-devel, jberring

On 2026-05-04 12:34:48 [-0400], Waiman Long wrote:
> The atomic_tx_lock was first introduced by commit 1abb081e41a7
> ("firmware: tegra: Simplify channel management") as a spinlock_t. It
> is used only in tegra_bpmp_transfer_atomic() to ensure mutual exclusion.
> 
> Since the merging of PREEMPT_RT support into mainline Linux kernel
> in v6.12, a spinlock becomes a sleeping lock when CONFIG_PREEMPT_RT
> is enabled. As tegra_bpmp_transfer_atomic() is called with interrupt
> disabled, acquiring a sleeping lock will lead to the following bug
> report when booting up a PREEMPT_RT kernel on an tegra based arm64
> system with Boot and Power Management Processor (BPMP).
> 
>   BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
>   in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0
>   preempt_count: 0, expected: 0
>   RCU nest depth: 0, expected: 0
>   2 locks held by swapper/0/1:
>    #0: ffff42bda2d5b0f0 (&dev->mutex){....}-{3:3}, at: __driver_attach+0x110/0x2c0
>    #1: ffff42bda4da5138 (&bpmp->atomic_tx_lock){....}-{2:2}, at: tegra_bpmp_transfer_atomic+0x118/0x3c0
> 
> Fix it by changing the type of atomic_tx_lock in the tegra_bpmp structure
> to raw_spinlock_t and use raw_spinlock APIs to access it.

Do you have a backtrace why interrupts are disabled to begin with?

Sebastian

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] firmware: tegra: bpmp: Make atomic_tx_lock a raw_spinlock
       [not found]   ` <afdb5fc1-f478-4547-aa39-04d477854d64@redhat.com>
@ 2026-05-06  6:41     ` Sebastian Andrzej Siewior
  0 siblings, 0 replies; 3+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-05-06  6:41 UTC (permalink / raw)
  To: Waiman Long
  Cc: Thierry Reding, Jonathan Hunter, Clark Williams, Steven Rostedt,
	linux-tegra, linux-kernel, linux-rt-devel, jberring

On 2026-05-05 15:47:23 [-0400], Waiman Long wrote:
>  __might_resched+0x254/0x330
>  rt_spin_lock+0x70/0x140
>  tegra_bpmp_transfer_atomic+0x118/0x3c0
>  tegra_bpmp_probe+0x564/0x6f0

So this is tegra_bpmp_ping().

> I know that interrupt is disabled becasue of the following code.
> 
> 346 int tegra_bpmp_transfer_atomic(struct tegra_bpmp *bpmp, 347 struct
> tegra_bpmp_message *msg) 348 { 349 struct tegra_bpmp_channel *channel; 350
> int err; 351 352 if (WARN_ON(!irqs_disabled())) 353 return -EPERM;

Well, yes. It disables interrupts just probably to document the time it
took for the transfer so it can write it then via dev_dbg().
It is hard to tell what the worst-case delay here is but it is probably
not important if this is just boot time/ driver probe. Maybe.
What I am bit more concerned if the actual path of this
tegra_bpmp_transfer_atomic() invocation via i2c driver is actually
invoked with disabled interrupts on PREEMPT_RT. Because that might not
be the case. I've been looking at the i2c call chain and it is not
obvious what the actual call chain is. There is just
i2c_in_atomic_xfer_mode() check. So it may or may not be used in the
end.

> Cheers, Longman

Sebastian

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-06  6:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-04 16:34 [PATCH] firmware: tegra: bpmp: Make atomic_tx_lock a raw_spinlock Waiman Long
2026-05-05  6:27 ` Sebastian Andrzej Siewior
     [not found]   ` <afdb5fc1-f478-4547-aa39-04d477854d64@redhat.com>
2026-05-06  6:41     ` Sebastian Andrzej Siewior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox