[PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[Russell King (Oracle)]

i2c-tegra marks its runtime PM as IRQ safe using pm_runtime_irq_safe().
However, tegra_i2c_runtime_suspend() calls
pinctrl_pm_select_idle_state(), which eventually calls
pinmux_disable_setting() which will take the desc->mux_lock mutex.
When this happens, the result is:

BUG: sleeping function called from invalid context at kernel/locking/mutex.c:591
in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 12, name: kworker/u24:0
preempt_count: 0, expected: 0
RCU nest depth: 0, expected: 0
3 locks held by kworker/u24:0/12:
 #0: ffff000080020d48 ((wq_completion)events_unbound#2){+.+.}-{0:0}, at: process_one_work+0x184/0x628
 #1: ffff80008225bde8 (deferred_probe_work){+.+.}-{0:0}, at: process_one_work+0x1ac/0x628
 #2: ffff000080ad90f8 (&dev->mutex){....}-{4:4}, at: __device_attach+0x2c/0x188
irq event stamp: 97058
ktime_get+0x130/0x180
_raw_spin_lock_irqsave+0x84/0x88
handle_softirqs+0x448/0x494
__do_softirq+0x14/0x20
CPU: 1 UID: 0 PID: 12 Comm: kworker/u24:0 Not tainted 6.19.0-rc8-net-next+ #591 PREEMPT
Hardware name: NVIDIA NVIDIA Jetson Xavier NX Developer Kit/Jetson, BIOS 6.0-37391689 08/28/2024
Workqueue: events_unbound deferred_probe_work_func
Call trace:
 show_stack+0x18/0x30 (C)
 dump_stack_lvl+0x6c/0x94
 dump_stack+0x18/0x24
 __might_resched+0x154/0x220
 __might_sleep+0x48/0x80
 __mutex_lock+0x48/0x800
 mutex_lock_nested+0x24/0x30
 pinmux_disable_setting+0x9c/0x180
 pinctrl_commit_state+0x5c/0x260
 pinctrl_pm_select_idle_state+0x4c/0xa0
 tegra_i2c_runtime_suspend+0x2c/0x3c
 pm_generic_runtime_suspend+0x2c/0x44
 __rpm_callback+0x48/0x1ec
 rpm_callback+0x74/0x80
 rpm_suspend+0xec/0x630
 rpm_idle+0x274/0x42c
 __pm_runtime_idle+0x44/0x154
 tegra_i2c_probe+0x2c0/0x540
 platform_probe+0x5c/0xa4
 really_probe+0xbc/0x2c0
 __driver_probe_device+0x78/0x120
 driver_probe_device+0x3c/0x160
 __device_attach_driver+0xb8/0x140
 bus_for_each_drv+0x70/0xb8
 __device_attach+0xa4/0x188
 device_initial_probe+0x50/0x54
 bus_probe_device+0x38/0xa4
 deferred_probe_work_func+0x90/0xcc
 process_one_work+0x204/0x628
 worker_thread+0x1bc/0x360
 kthread+0x138/0x210
 ret_from_fork+0x10/0x20

This was observed on the nVidia Jetson Xavier NX platform.

Thus, no, the runtime PM is not IRQ-safe. Remove the call marking it as
such.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
The IRQ-safe marking was introduced by commit ede2299f7101 ("i2c:
tegra: Support atomic transfers").

However, since then there have been patches disabling the IRQ-safe
marking:

9e29420ddb13 i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM
14d069d92951 i2c: tegra: Do not mark ACPI devices as irq safe

Clearly, the presence of pinctrl_pm_select_idle_state() which can sleep
means, definitively, that runtime PM on this device is not IRQ safe,
and if the original patch introducing atomic transfers relies on these
being IRQ safe, that patch was incorrect (maybe on such devices, it
should not change the pin state, and the driver should have a flag to
allow the driver to be used in atomic contexts?)

The alternative to this patch is to get rid of the pinctrl calls in the
runtime PM path.
---
 drivers/i2c/busses/i2c-tegra.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index e533460bccc3..878a2d1c9749 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1829,18 +1829,6 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 	if (err)
 		goto release_clocks;
 
-	/*
-	 * VI I2C is in VE power domain which is not always ON and not
-	 * IRQ-safe.  Thus, IRQ-safe device shouldn't be attached to a
-	 * non IRQ-safe domain because this prevents powering off the power
-	 * domain.
-	 *
-	 * VI I2C device shouldn't be marked as IRQ-safe because VI I2C won't
-	 * be used for atomic transfers. ACPI device is not IRQ safe also.
-	 */
-	if (!IS_VI(i2c_dev) && !has_acpi_companion(i2c_dev->dev))
-		pm_runtime_irq_safe(i2c_dev->dev);
-
 	pm_runtime_enable(i2c_dev->dev);
 
 	err = tegra_i2c_init_hardware(i2c_dev);
-- 
2.47.3

Re: [PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[Russell King (Oracle)]

Please ignore the net-next in the subject line, but yes, that was the
tree this patch was generated against.

On Tue, Feb 17, 2026 at 03:40:47PM +0000, Russell King (Oracle) wrote:
> i2c-tegra marks its runtime PM as IRQ safe using pm_runtime_irq_safe().
> However, tegra_i2c_runtime_suspend() calls
> pinctrl_pm_select_idle_state(), which eventually calls
> pinmux_disable_setting() which will take the desc->mux_lock mutex.
> When this happens, the result is:
> 
> BUG: sleeping function called from invalid context at kernel/locking/mutex.c:591
> in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 12, name: kworker/u24:0
> preempt_count: 0, expected: 0
> RCU nest depth: 0, expected: 0
> 3 locks held by kworker/u24:0/12:
>  #0: ffff000080020d48 ((wq_completion)events_unbound#2){+.+.}-{0:0}, at: process_one_work+0x184/0x628
>  #1: ffff80008225bde8 (deferred_probe_work){+.+.}-{0:0}, at: process_one_work+0x1ac/0x628
>  #2: ffff000080ad90f8 (&dev->mutex){....}-{4:4}, at: __device_attach+0x2c/0x188
> irq event stamp: 97058
> ktime_get+0x130/0x180
> _raw_spin_lock_irqsave+0x84/0x88
> handle_softirqs+0x448/0x494
> __do_softirq+0x14/0x20
> CPU: 1 UID: 0 PID: 12 Comm: kworker/u24:0 Not tainted 6.19.0-rc8-net-next+ #591 PREEMPT
> Hardware name: NVIDIA NVIDIA Jetson Xavier NX Developer Kit/Jetson, BIOS 6.0-37391689 08/28/2024
> Workqueue: events_unbound deferred_probe_work_func
> Call trace:
>  show_stack+0x18/0x30 (C)
>  dump_stack_lvl+0x6c/0x94
>  dump_stack+0x18/0x24
>  __might_resched+0x154/0x220
>  __might_sleep+0x48/0x80
>  __mutex_lock+0x48/0x800
>  mutex_lock_nested+0x24/0x30
>  pinmux_disable_setting+0x9c/0x180
>  pinctrl_commit_state+0x5c/0x260
>  pinctrl_pm_select_idle_state+0x4c/0xa0
>  tegra_i2c_runtime_suspend+0x2c/0x3c
>  pm_generic_runtime_suspend+0x2c/0x44
>  __rpm_callback+0x48/0x1ec
>  rpm_callback+0x74/0x80
>  rpm_suspend+0xec/0x630
>  rpm_idle+0x274/0x42c
>  __pm_runtime_idle+0x44/0x154
>  tegra_i2c_probe+0x2c0/0x540
>  platform_probe+0x5c/0xa4
>  really_probe+0xbc/0x2c0
>  __driver_probe_device+0x78/0x120
>  driver_probe_device+0x3c/0x160
>  __device_attach_driver+0xb8/0x140
>  bus_for_each_drv+0x70/0xb8
>  __device_attach+0xa4/0x188
>  device_initial_probe+0x50/0x54
>  bus_probe_device+0x38/0xa4
>  deferred_probe_work_func+0x90/0xcc
>  process_one_work+0x204/0x628
>  worker_thread+0x1bc/0x360
>  kthread+0x138/0x210
>  ret_from_fork+0x10/0x20
> 
> This was observed on the nVidia Jetson Xavier NX platform.
> 
> Thus, no, the runtime PM is not IRQ-safe. Remove the call marking it as
> such.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
> The IRQ-safe marking was introduced by commit ede2299f7101 ("i2c:
> tegra: Support atomic transfers").
> 
> However, since then there have been patches disabling the IRQ-safe
> marking:
> 
> 9e29420ddb13 i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM
> 14d069d92951 i2c: tegra: Do not mark ACPI devices as irq safe
> 
> Clearly, the presence of pinctrl_pm_select_idle_state() which can sleep
> means, definitively, that runtime PM on this device is not IRQ safe,
> and if the original patch introducing atomic transfers relies on these
> being IRQ safe, that patch was incorrect (maybe on such devices, it
> should not change the pin state, and the driver should have a flag to
> allow the driver to be used in atomic contexts?)
> 
> The alternative to this patch is to get rid of the pinctrl calls in the
> runtime PM path.
> ---
>  drivers/i2c/busses/i2c-tegra.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index e533460bccc3..878a2d1c9749 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -1829,18 +1829,6 @@ static int tegra_i2c_probe(struct platform_device *pdev)
>  	if (err)
>  		goto release_clocks;
>  
> -	/*
> -	 * VI I2C is in VE power domain which is not always ON and not
> -	 * IRQ-safe.  Thus, IRQ-safe device shouldn't be attached to a
> -	 * non IRQ-safe domain because this prevents powering off the power
> -	 * domain.
> -	 *
> -	 * VI I2C device shouldn't be marked as IRQ-safe because VI I2C won't
> -	 * be used for atomic transfers. ACPI device is not IRQ safe also.
> -	 */
> -	if (!IS_VI(i2c_dev) && !has_acpi_companion(i2c_dev->dev))
> -		pm_runtime_irq_safe(i2c_dev->dev);
> -
>  	pm_runtime_enable(i2c_dev->dev);
>  
>  	err = tegra_i2c_init_hardware(i2c_dev);
> -- 
> 2.47.3
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[Jon Hunter]

Hi Russell,

On 17/02/2026 15:40, Russell King (Oracle) wrote:
> i2c-tegra marks its runtime PM as IRQ safe using pm_runtime_irq_safe().
> However, tegra_i2c_runtime_suspend() calls
> pinctrl_pm_select_idle_state(), which eventually calls
> pinmux_disable_setting() which will take the desc->mux_lock mutex.
> When this happens, the result is:
> 
> BUG: sleeping function called from invalid context at kernel/locking/mutex.c:591
> in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 12, name: kworker/u24:0
> preempt_count: 0, expected: 0
> RCU nest depth: 0, expected: 0
> 3 locks held by kworker/u24:0/12:
>   #0: ffff000080020d48 ((wq_completion)events_unbound#2){+.+.}-{0:0}, at: process_one_work+0x184/0x628
>   #1: ffff80008225bde8 (deferred_probe_work){+.+.}-{0:0}, at: process_one_work+0x1ac/0x628
>   #2: ffff000080ad90f8 (&dev->mutex){....}-{4:4}, at: __device_attach+0x2c/0x188
> irq event stamp: 97058
> ktime_get+0x130/0x180
> _raw_spin_lock_irqsave+0x84/0x88
> handle_softirqs+0x448/0x494
> __do_softirq+0x14/0x20
> CPU: 1 UID: 0 PID: 12 Comm: kworker/u24:0 Not tainted 6.19.0-rc8-net-next+ #591 PREEMPT
> Hardware name: NVIDIA NVIDIA Jetson Xavier NX Developer Kit/Jetson, BIOS 6.0-37391689 08/28/2024
> Workqueue: events_unbound deferred_probe_work_func
> Call trace:
>   show_stack+0x18/0x30 (C)
>   dump_stack_lvl+0x6c/0x94
>   dump_stack+0x18/0x24
>   __might_resched+0x154/0x220
>   __might_sleep+0x48/0x80
>   __mutex_lock+0x48/0x800
>   mutex_lock_nested+0x24/0x30
>   pinmux_disable_setting+0x9c/0x180
>   pinctrl_commit_state+0x5c/0x260
>   pinctrl_pm_select_idle_state+0x4c/0xa0
>   tegra_i2c_runtime_suspend+0x2c/0x3c
>   pm_generic_runtime_suspend+0x2c/0x44
>   __rpm_callback+0x48/0x1ec
>   rpm_callback+0x74/0x80
>   rpm_suspend+0xec/0x630
>   rpm_idle+0x274/0x42c
>   __pm_runtime_idle+0x44/0x154
>   tegra_i2c_probe+0x2c0/0x540
>   platform_probe+0x5c/0xa4
>   really_probe+0xbc/0x2c0
>   __driver_probe_device+0x78/0x120
>   driver_probe_device+0x3c/0x160
>   __device_attach_driver+0xb8/0x140
>   bus_for_each_drv+0x70/0xb8
>   __device_attach+0xa4/0x188
>   device_initial_probe+0x50/0x54
>   bus_probe_device+0x38/0xa4
>   deferred_probe_work_func+0x90/0xcc
>   process_one_work+0x204/0x628
>   worker_thread+0x1bc/0x360
>   kthread+0x138/0x210
>   ret_from_fork+0x10/0x20
> 
> This was observed on the nVidia Jetson Xavier NX platform.
> 
> Thus, no, the runtime PM is not IRQ-safe. Remove the call marking it as
> such.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
> The IRQ-safe marking was introduced by commit ede2299f7101 ("i2c:
> tegra: Support atomic transfers").
> 
> However, since then there have been patches disabling the IRQ-safe
> marking:
> 
> 9e29420ddb13 i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM
> 14d069d92951 i2c: tegra: Do not mark ACPI devices as irq safe
> 
> Clearly, the presence of pinctrl_pm_select_idle_state() which can sleep
> means, definitively, that runtime PM on this device is not IRQ safe,
> and if the original patch introducing atomic transfers relies on these
> being IRQ safe, that patch was incorrect (maybe on such devices, it
> should not change the pin state, and the driver should have a flag to
> allow the driver to be used in atomic contexts?)
> 
> The alternative to this patch is to get rid of the pinctrl calls in the
> runtime PM path.


Mikko recently posted this fix [0]. Hopefully, this also works?

Thanks
Jon

[0] 
https://lore.kernel.org/linux-tegra/20260217-i2c-dpaux-irqsafe-v2-1-635a4c43b1a7@nvidia.com/T/#u

-- 
nvpublic

Re: [PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[Russell King (Oracle)]

On Tue, Feb 17, 2026 at 03:55:47PM +0000, Jon Hunter wrote:
> Hi Russell,
> 
> Mikko recently posted this fix [0]. Hopefully, this also works?
> 
> Thanks
> Jon
> 
> [0] https://lore.kernel.org/linux-tegra/20260217-i2c-dpaux-irqsafe-v2-1-635a4c43b1a7@nvidia.com/T/#u

Looks like it probably would do, but I note that it fixes two problems.
What happened to "fix one problem with one patch" ?

From Documentation/process/submitting-patches.rst:

  Solve only one problem per patch.  If your description starts to get
  long, that's a sign that you probably need to split up your patch.
  See :ref:`split_changes`.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[Jon Hunter]

On 17/02/2026 16:04, Russell King (Oracle) wrote:
> On Tue, Feb 17, 2026 at 03:55:47PM +0000, Jon Hunter wrote:
>> Hi Russell,
>>
>> Mikko recently posted this fix [0]. Hopefully, this also works?
>>
>> Thanks
>> Jon
>>
>> [0] https://lore.kernel.org/linux-tegra/20260217-i2c-dpaux-irqsafe-v2-1-635a4c43b1a7@nvidia.com/T/#u
> 
> Looks like it probably would do, but I note that it fixes two problems.
> What happened to "fix one problem with one patch" ?
> 
>  From Documentation/process/submitting-patches.rst:
> 
>    Solve only one problem per patch.  If your description starts to get
>    long, that's a sign that you probably need to split up your patch.
>    See :ref:`split_changes`.

Yes we should always follow that rule. However, in this case, I believe 
that the build time dependency on the PINCTRL subsystem was only exposed 
by adding the 'i2c_dev->dev->pins'. Unless I am misunderstanding ...

Jon

-- 
nvpublic

Re: [PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[Russell King (Oracle)]

On Tue, Feb 17, 2026 at 04:15:07PM +0000, Jon Hunter wrote:
> 
> On 17/02/2026 16:04, Russell King (Oracle) wrote:
> > On Tue, Feb 17, 2026 at 03:55:47PM +0000, Jon Hunter wrote:
> > > Hi Russell,
> > > 
> > > Mikko recently posted this fix [0]. Hopefully, this also works?
> > > 
> > > Thanks
> > > Jon
> > > 
> > > [0] https://lore.kernel.org/linux-tegra/20260217-i2c-dpaux-irqsafe-v2-1-635a4c43b1a7@nvidia.com/T/#u
> > 
> > Looks like it probably would do, but I note that it fixes two problems.
> > What happened to "fix one problem with one patch" ?
> > 
> >  From Documentation/process/submitting-patches.rst:
> > 
> >    Solve only one problem per patch.  If your description starts to get
> >    long, that's a sign that you probably need to split up your patch.
> >    See :ref:`split_changes`.
> 
> Yes we should always follow that rule. However, in this case, I believe that
> the build time dependency on the PINCTRL subsystem was only exposed by
> adding the 'i2c_dev->dev->pins'. Unless I am misunderstanding ...

Yes, it looks like it.

However, I wonder why the dependency has to be complicated.

ARCH_TEGRA in both arm64 and arm selects PINCTRL, so we can assume that
PINCTRL will be set for ARCH_TEGRA. So:

 config I2C_TEGRA
 	tristate "NVIDIA Tegra internal I2C controller"
 	depends on ARCH_TEGRA || (COMPILE_TEST && (ARC || ARM || ARM64 || M68K || RISCV || SUPERH || SPARC))
+	depends on PINCTRL

is a shorter way of writing this, and it makes sense - pinctrl isn't
required because we're doing a compile test, it's required because
the driver itself fundamentally requires it with this change whether
or not we're doing a compile test.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[Jon Hunter]

On 17/02/2026 16:40, Russell King (Oracle) wrote:

...

>> Yes we should always follow that rule. However, in this case, I believe that
>> the build time dependency on the PINCTRL subsystem was only exposed by
>> adding the 'i2c_dev->dev->pins'. Unless I am misunderstanding ...
> 
> Yes, it looks like it.
> 
> However, I wonder why the dependency has to be complicated.
> 
> ARCH_TEGRA in both arm64 and arm selects PINCTRL, so we can assume that
> PINCTRL will be set for ARCH_TEGRA. So:
> 
>   config I2C_TEGRA
>   	tristate "NVIDIA Tegra internal I2C controller"
>   	depends on ARCH_TEGRA || (COMPILE_TEST && (ARC || ARM || ARM64 || M68K || RISCV || SUPERH || SPARC))
> +	depends on PINCTRL
> 
> is a shorter way of writing this, and it makes sense - pinctrl isn't
> required because we're doing a compile test, it's required because
> the driver itself fundamentally requires it with this change whether
> or not we're doing a compile test.

Yes that's true indeed.

Mikko, do you want to take care of this?

Jon

-- 
nvpublic

Re: [PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[Mikko Perttunen]

On Wednesday, February 18, 2026 1:46 AM Jon Hunter wrote:
> On 17/02/2026 16:40, Russell King (Oracle) wrote:
> 
> ...
> 
> >> Yes we should always follow that rule. However, in this case, I believe
> >> that the build time dependency on the PINCTRL subsystem was only exposed
> >> by adding the 'i2c_dev->dev->pins'. Unless I am misunderstanding ...
> > 
> > Yes, it looks like it.
> > 
> > However, I wonder why the dependency has to be complicated.
> > 
> > ARCH_TEGRA in both arm64 and arm selects PINCTRL, so we can assume that
> > 
> > PINCTRL will be set for ARCH_TEGRA. So:
> >   config I2C_TEGRA
> >   
> >   	tristate "NVIDIA Tegra internal I2C controller"
> >   	depends on ARCH_TEGRA || (COMPILE_TEST && (ARC || ARM || ARM64 || 
M68K
> >   	|| RISCV || SUPERH || SPARC))> 
> > +	depends on PINCTRL
> > 
> > is a shorter way of writing this, and it makes sense - pinctrl isn't
> > required because we're doing a compile test, it's required because
> > the driver itself fundamentally requires it with this change whether
> > or not we're doing a compile test.
> 
> Yes that's true indeed.
> 
> Mikko, do you want to take care of this?

My thought was it would be better to keep the PINCTRL dependency grouped with 
COMPILE_TEST. That makes it clear it's only needed because of it -- clearer to 
the reader that ARCH_TEGRA implies it. Kind of like not checking for NULL 
pointers in C code when the contract is that the pointer is not NULL.

I can change it though if you'd like.

Mikko

> 
> Jon
> 
> --
> nvpublic

Re: [PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[Jon Hunter]

On 18/02/2026 01:35, Mikko Perttunen wrote:

...

>>>> Yes we should always follow that rule. However, in this case, I believe
>>>> that the build time dependency on the PINCTRL subsystem was only exposed
>>>> by adding the 'i2c_dev->dev->pins'. Unless I am misunderstanding ...
>>>
>>> Yes, it looks like it.
>>>
>>> However, I wonder why the dependency has to be complicated.
>>>
>>> ARCH_TEGRA in both arm64 and arm selects PINCTRL, so we can assume that
>>>
>>> PINCTRL will be set for ARCH_TEGRA. So:
>>>    config I2C_TEGRA
>>>    
>>>    	tristate "NVIDIA Tegra internal I2C controller"
>>>    	depends on ARCH_TEGRA || (COMPILE_TEST && (ARC || ARM || ARM64 ||
> M68K
>>>    	|| RISCV || SUPERH || SPARC))>
>>> +	depends on PINCTRL
>>>
>>> is a shorter way of writing this, and it makes sense - pinctrl isn't
>>> required because we're doing a compile test, it's required because
>>> the driver itself fundamentally requires it with this change whether
>>> or not we're doing a compile test.
>>
>> Yes that's true indeed.
>>
>> Mikko, do you want to take care of this?
> 
> My thought was it would be better to keep the PINCTRL dependency grouped with
> COMPILE_TEST. That makes it clear it's only needed because of it -- clearer to
> the reader that ARCH_TEGRA implies it. Kind of like not checking for NULL
> pointers in C code when the contract is that the pointer is not NULL.

Russell's point is that regardless of the compile test, the driver has a 
dependency on pinctrl and so should always be dependent on it. The I2C 
instances for the DPAUX device on certain devices require this and will 
not work without it (before your change was added). I guess I should 
have added this dependency back with commit 718917b9875f ("i2c: tegra: 
Add pinctrl support").

> I can change it though if you'd like.

I think we should.

Thanks!
Jon

-- 
nvpublic

Re: [PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[Russell King (Oracle)]

On Wed, Feb 18, 2026 at 08:30:49AM +0000, Jon Hunter wrote:
> 
> On 18/02/2026 01:35, Mikko Perttunen wrote:
> 
> ...
> 
> > > > > Yes we should always follow that rule. However, in this case, I believe
> > > > > that the build time dependency on the PINCTRL subsystem was only exposed
> > > > > by adding the 'i2c_dev->dev->pins'. Unless I am misunderstanding ...
> > > > 
> > > > Yes, it looks like it.
> > > > 
> > > > However, I wonder why the dependency has to be complicated.
> > > > 
> > > > ARCH_TEGRA in both arm64 and arm selects PINCTRL, so we can assume that
> > > > 
> > > > PINCTRL will be set for ARCH_TEGRA. So:
> > > >    config I2C_TEGRA
> > > >    	tristate "NVIDIA Tegra internal I2C controller"
> > > >    	depends on ARCH_TEGRA || (COMPILE_TEST && (ARC || ARM || ARM64 ||
> > M68K
> > > >    	|| RISCV || SUPERH || SPARC))>
> > > > +	depends on PINCTRL
> > > > 
> > > > is a shorter way of writing this, and it makes sense - pinctrl isn't
> > > > required because we're doing a compile test, it's required because
> > > > the driver itself fundamentally requires it with this change whether
> > > > or not we're doing a compile test.
> > > 
> > > Yes that's true indeed.
> > > 
> > > Mikko, do you want to take care of this?
> > 
> > My thought was it would be better to keep the PINCTRL dependency grouped with
> > COMPILE_TEST. That makes it clear it's only needed because of it -- clearer to
> > the reader that ARCH_TEGRA implies it. Kind of like not checking for NULL
> > pointers in C code when the contract is that the pointer is not NULL.
> 
> Russell's point is that regardless of the compile test, the driver has a
> dependency on pinctrl and so should always be dependent on it. The I2C
> instances for the DPAUX device on certain devices require this and will not
> work without it (before your change was added). I guess I should have added
> this dependency back with commit 718917b9875f ("i2c: tegra: Add pinctrl
> support").
> 
> > I can change it though if you'd like.
> 
> I think we should.
> 
> Thanks!

When is this bug going to be fixed? This is a regression that's
affecting Xavier systems. It's been over a month since I proposed
a patch to fix this:

BUG: sleeping function called from invalid context at kernel/locking/mutex.c:591
in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 59, name: kworker/u24:6

Please see:

https://lore.kernel.org/r/E1vsNBv-00000009nfA-27ZK@rmk-PC.armlinux.org.uk

for my original proposed fix, complete kernel messages and analysis.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] i2c: tegra: runtime PM is not IRQ-safe

[Mikko Perttunen]

On Thursday, March 19, 2026 3:17 AM Russell King (Oracle) wrote:
> On Wed, Feb 18, 2026 at 08:30:49AM +0000, Jon Hunter wrote:
> > On 18/02/2026 01:35, Mikko Perttunen wrote:
> > 
> > ...
> > 
> > > > > > Yes we should always follow that rule. However, in this case, I
> > > > > > believe
> > > > > > that the build time dependency on the PINCTRL subsystem was only
> > > > > > exposed
> > > > > > by adding the 'i2c_dev->dev->pins'. Unless I am misunderstanding
> > > > > > ...
> > > > > 
> > > > > Yes, it looks like it.
> > > > > 
> > > > > However, I wonder why the dependency has to be complicated.
> > > > > 
> > > > > ARCH_TEGRA in both arm64 and arm selects PINCTRL, so we can assume
> > > > > that
> > > > > 
> > > > > PINCTRL will be set for ARCH_TEGRA. So:
> > > > >    config I2C_TEGRA
> > > > >    
> > > > >    	tristate "NVIDIA Tegra internal I2C controller"
> > > > >    	depends on ARCH_TEGRA || (COMPILE_TEST && (ARC || ARM || 
ARM64
> > > > >    	||
> > > 
> > > M68K
> > > 
> > > > >    	|| RISCV || SUPERH || SPARC))>
> > > > > 
> > > > > +	depends on PINCTRL
> > > > > 
> > > > > is a shorter way of writing this, and it makes sense - pinctrl isn't
> > > > > required because we're doing a compile test, it's required because
> > > > > the driver itself fundamentally requires it with this change whether
> > > > > or not we're doing a compile test.
> > > > 
> > > > Yes that's true indeed.
> > > > 
> > > > Mikko, do you want to take care of this?
> > > 
> > > My thought was it would be better to keep the PINCTRL dependency grouped
> > > with COMPILE_TEST. That makes it clear it's only needed because of it
> > > -- clearer to the reader that ARCH_TEGRA implies it. Kind of like not
> > > checking for NULL pointers in C code when the contract is that the
> > > pointer is not NULL.> 
> > Russell's point is that regardless of the compile test, the driver has a
> > dependency on pinctrl and so should always be dependent on it. The I2C
> > instances for the DPAUX device on certain devices require this and will
> > not
> > work without it (before your change was added). I guess I should have
> > added
> > this dependency back with commit 718917b9875f ("i2c: tegra: Add pinctrl
> > support").
> > 
> > > I can change it though if you'd like.
> > 
> > I think we should.
> > 
> > Thanks!
> 
> When is this bug going to be fixed? This is a regression that's
> affecting Xavier systems. It's been over a month since I proposed
> a patch to fix this:
> 
> BUG: sleeping function called from invalid context at
> kernel/locking/mutex.c:591 in_atomic(): 0, irqs_disabled(): 1, non_block:
> 0, pid: 59, name: kworker/u24:6
> 
> Please see:
> 
> https://lore.kernel.org/r/E1vsNBv-00000009nfA-27ZK@rmk-PC.armlinux.org.uk
> 
> for my original proposed fix, complete kernel messages and analysis.
> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

FWIW I did post v3 of my patch a while back: https://lore.kernel.org/linux-tegra/20260303-i2c-dpaux-irqsafe-v3-1-75ca95b96666@nvidia.com/

Mikko