[PATCH] firmware: tegra: bpmp: Make atomic_tx_lock a raw_spinlock

[PATCH] firmware: tegra: bpmp: Make atomic_tx_lock a raw_spinlock

[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

Re: [PATCH] firmware: tegra: bpmp: Make atomic_tx_lock a raw_spinlock

[Sebastian Andrzej Siewior]

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

Re: [PATCH] firmware: tegra: bpmp: Make atomic_tx_lock a raw_spinlock

[Sebastian Andrzej Siewior]

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

Re: [PATCH] firmware: tegra: bpmp: Make atomic_tx_lock a raw_spinlock

[Waiman Long]

On 5/6/26 2:41 AM, Sebastian Andrzej Siewior wrote:
> 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.

tegra_bpmp_channel_write() calls tegra_bpmp_wait_request_channel_free() 
which waits until the channel is freed. So it is hard to tell how long 
does that take. In my case, the bug report happened at boot time.

Anyway, are there other comment about this patch?

Thanks,
Longman