Received-SPF: pass (google.com: domain of linux-kernel+bounces-209878-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 11 Jun 2024 15:49:02 +0300
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Breno Leitao <leitao@debian.org>
Cc: Laxman Dewangan <ldewangan@nvidia.com>,
	Dmitry Osipenko <digetx@gmail.com>,
	Andi Shyti <andi.shyti@kernel.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>, paulmck@kernel.org,
	apopple@nvidia.com, Michael van der Westhuizen <rmikey@meta.com>,
	"open list:I2C SUBSYSTEM HOST DRIVERS" <linux-i2c@vger.kernel.org>,
	"open list:TEGRA ARCHITECTURE SUPPORT" <linux-tegra@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] [i2c-tegra] Do not mark ACPI devices as irq safe
Message-ID: <ZmhHvpHlkxe4kid7@smile.fi.intel.com>
References: <20240606132708.1610308-1-leitao@debian.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240606132708.1610308-1-leitao@debian.org>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

On Thu, Jun 06, 2024 at 06:27:07AM -0700, Breno Leitao wrote:
> On ACPI machines, the tegra i2c module encounters an issue due to a
> mutex being called inside a spinlock. This leads to the following bug:
> 
> 	BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585
> 	in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1282, name: kssif0010
> 	preempt_count: 0, expected: 0
> 	RCU nest depth: 0, expected: 0
> 	irq event stamp: 0
> 
> 	Call trace:
> 	dump_backtrace+0xf0/0x140
> 	show_stack (./arch/x86/include/asm/current.h:49
> 		     arch/x86/kernel/dumpstack.c:312)
> 	dump_stack_lvl (lib/dump_stack.c:89 lib/dump_stack.c:115)
> 	dump_stack (lib/earlycpio.c:61)
> 	__might_resched (./arch/x86/include/asm/current.h:49
> 			 kernel/sched/core.c:10297)
> 	__might_sleep (./include/linux/lockdep.h:231
> 			 kernel/sched/core.c:10236)
> 	__mutex_lock_common+0x5c/0x2190
> 	mutex_lock_nested (kernel/locking/mutex.c:751)
> 	acpi_subsys_runtime_resume+0xb8/0x160
> 	__rpm_callback+0x1cc/0x4b0
> 	rpm_resume+0xa60/0x1078
> 	__pm_runtime_resume+0xbc/0x130
> 	tegra_i2c_xfer+0x74/0x398
> 	__i2c_transfer (./include/trace/events/i2c.h:122 drivers/i2c/i2c-core-base.c:2258)

Please, read
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
and follow the advice on how to improve your commit message.

> The problem arises because during __pm_runtime_resume(), the spinlock
> &dev->power.lock is acquired before rpm_resume() is called. Later,
> rpm_resume() invokes acpi_subsys_runtime_resume(), which relies on
> mutexes, triggering the error.
> 
> To address this issue, devices on ACPI are now marked as not IRQ-safe,
> considering the dependency of acpi_subsys_runtime_resume() on mutexes.

...

While it's a move in the right direction, the real fix is to get rid of
the IRQ safe PM hack completely. Look at how OMAP code was modified for
the last few years and now it's pm_runtime_irq_safe()-free. The main
(ab)users are SH code followed by Tegra drivers.

So, can we fix this once for all?

-- 
With Best Regards,
Andy Shevchenko