Hi!
The Tegra20 requires an enabled VDE power domain during startup. As the VDE is currently not used, it is disabled during runtime. Since 8f0c714ad9be, there is a workaround for the "normal restart path" which enables the VDE before doing PMC's warm reboot. This workaround is not executed in the "emergency restart path", leading to a hang-up during start.
This series implements and registers a new pmic-based restart handler for boards with tps6586x. This cold reboot ensures that the VDE power domain is enabled during startup on tegra20-based boards.
Since bae1d3a05a8b, i2c transfers are non-atomic while preemption is disabled (which is e.g. done during panic()). This could lead to warnings ("Voluntary context switch within RCU") in i2c-based restart handlers during emergency restart. The state of preemption should be detected by i2c_in_atomic_xfer_mode() to use atomic i2c xfer when required. Beside the new system_state check, the check is the same as the one pre v5.2.
--- v7: - 5/5: drop mode check (suggested by Dmitry) - Link to v6: https://lore.kernel.org/r/20230327-tegra-pmic-reboot-v6-0-af44a4cd82e9@skida...
v6: - drop 4/6 to abort restart on unexpected failure (suggested by Dmitry) - 4,5: fix comments in handlers (suggested by Lee) - 4,5: same delay for both handlers (suggested by Lee)
v5: - introduce new 3 & 4, therefore 3 -> 5, 4 -> 6 - 3: provide dev to sys_off handler, if it is known - 4: return NOTIFY_DONE from sys_off_notify, to avoid skipping - 5: drop Reviewed-by from Dmitry, add poweroff timeout - 5,6: return notifier value instead of direct errno from handler - 5,6: use new dev field instead of passing dev as cb_data - 5,6: increase timeout values based on error observations - 6: skip unsupported reboot modes in restart handler
--- Benjamin Bara (5): kernel/reboot: emergency_restart: set correct system_state i2c: core: run atomic i2c xfer when !preemptible kernel/reboot: add device to sys_off_handler mfd: tps6586x: use devm-based power off handler mfd: tps6586x: register restart handler
drivers/i2c/i2c-core.h | 2 +- drivers/mfd/tps6586x.c | 50 ++++++++++++++++++++++++++++++++++++++++++-------- include/linux/reboot.h | 3 +++ kernel/reboot.c | 4 ++++ 4 files changed, 50 insertions(+), 9 deletions(-) --- base-commit: 197b6b60ae7bc51dd0814953c562833143b292aa change-id: 20230327-tegra-pmic-reboot-4175ff814a4b
Best regards,
From: Benjamin Bara benjamin.bara@skidata.com
As the emergency restart does not call kernel_restart_prepare(), the system_state stays in SYSTEM_RUNNING.
Since bae1d3a05a8b, this hinders i2c_in_atomic_xfer_mode() from becoming active, and therefore might lead to avoidable warnings in the restart handlers, e.g.:
[ 12.667612] WARNING: CPU: 1 PID: 1 at kernel/rcu/tree_plugin.h:318 rcu_note_context_switch+0x33c/0x6b0 [ 12.676926] Voluntary context switch within RCU read-side critical section! ... [ 12.742376] schedule_timeout from wait_for_completion_timeout+0x90/0x114 [ 12.749179] wait_for_completion_timeout from tegra_i2c_wait_completion+0x40/0x70 ... [ 12.994527] atomic_notifier_call_chain from machine_restart+0x34/0x58 [ 13.001050] machine_restart from panic+0x2a8/0x32c
Avoid these by setting the correct system_state.
Fixes: bae1d3a05a8b ("i2c: core: remove use of in_atomic()") Cc: stable@vger.kernel.org # v5.2+ Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com Tested-by: Nishanth Menon nm@ti.com Signed-off-by: Benjamin Bara benjamin.bara@skidata.com --- kernel/reboot.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/kernel/reboot.c b/kernel/reboot.c index 3bba88c7ffc6..6ebef11c8876 100644 --- a/kernel/reboot.c +++ b/kernel/reboot.c @@ -74,6 +74,7 @@ void __weak (*pm_power_off)(void); void emergency_restart(void) { kmsg_dump(KMSG_DUMP_EMERG); + system_state = SYSTEM_RESTART; machine_emergency_restart(); } EXPORT_SYMBOL_GPL(emergency_restart);
From: Benjamin Bara benjamin.bara@skidata.com
Since bae1d3a05a8b, i2c transfers are non-atomic if preemption is disabled. However, non-atomic i2c transfers require preemption (e.g. in wait_for_completion() while waiting for the DMA).
panic() calls preempt_disable_notrace() before calling emergency_restart(). Therefore, if an i2c device is used for the restart, the xfer should be atomic. This avoids warnings like:
[ 12.667612] WARNING: CPU: 1 PID: 1 at kernel/rcu/tree_plugin.h:318 rcu_note_context_switch+0x33c/0x6b0 [ 12.676926] Voluntary context switch within RCU read-side critical section! ... [ 12.742376] schedule_timeout from wait_for_completion_timeout+0x90/0x114 [ 12.749179] wait_for_completion_timeout from tegra_i2c_wait_completion+0x40/0x70 ... [ 12.994527] atomic_notifier_call_chain from machine_restart+0x34/0x58 [ 13.001050] machine_restart from panic+0x2a8/0x32c
Use !preemptible() instead, which is basically the same check as pre-v5.2.
Fixes: bae1d3a05a8b ("i2c: core: remove use of in_atomic()") Cc: stable@vger.kernel.org # v5.2+ Suggested-by: Dmitry Osipenko dmitry.osipenko@collabora.com Acked-by: Wolfram Sang wsa@kernel.org Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com Tested-by: Nishanth Menon nm@ti.com Signed-off-by: Benjamin Bara benjamin.bara@skidata.com --- drivers/i2c/i2c-core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h index 1247e6e6e975..05b8b8dfa9bd 100644 --- a/drivers/i2c/i2c-core.h +++ b/drivers/i2c/i2c-core.h @@ -29,7 +29,7 @@ int i2c_dev_irq_from_resources(const struct resource *resources, */ static inline bool i2c_in_atomic_xfer_mode(void) { - return system_state > SYSTEM_RUNNING && irqs_disabled(); + return system_state > SYSTEM_RUNNING && !preemptible(); }
static inline int __i2c_lock_bus_helper(struct i2c_adapter *adap)
On Sat, Jul 15, 2023 at 09:53:24AM +0200, Benjamin Bara wrote:
From: Benjamin Bara benjamin.bara@skidata.com
Since bae1d3a05a8b, i2c transfers are non-atomic if preemption is disabled. However, non-atomic i2c transfers require preemption (e.g. in wait_for_completion() while waiting for the DMA).
panic() calls preempt_disable_notrace() before calling emergency_restart(). Therefore, if an i2c device is used for the restart, the xfer should be atomic. This avoids warnings like:
[ 12.667612] WARNING: CPU: 1 PID: 1 at kernel/rcu/tree_plugin.h:318 rcu_note_context_switch+0x33c/0x6b0 [ 12.676926] Voluntary context switch within RCU read-side critical section! ... [ 12.742376] schedule_timeout from wait_for_completion_timeout+0x90/0x114 [ 12.749179] wait_for_completion_timeout from tegra_i2c_wait_completion+0x40/0x70 ... [ 12.994527] atomic_notifier_call_chain from machine_restart+0x34/0x58 [ 13.001050] machine_restart from panic+0x2a8/0x32c
Use !preemptible() instead, which is basically the same check as pre-v5.2.
Fixes: bae1d3a05a8b ("i2c: core: remove use of in_atomic()") Cc: stable@vger.kernel.org # v5.2+ Suggested-by: Dmitry Osipenko dmitry.osipenko@collabora.com Acked-by: Wolfram Sang wsa@kernel.org Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com Tested-by: Nishanth Menon nm@ti.com Signed-off-by: Benjamin Bara benjamin.bara@skidata.com
For kernel 6.7 I'm having an issue when I shutdown or reboot my Rockchip RK3326 or Rockchip RK3566 based devices, and I've bisected the issue down to this specific commit.
When I shutdown or restart the device, I receive messages in the kernel log like the following:
[ 37.121148] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.122178] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.123212] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.124226] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.125242] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.126133] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x1
The device will also occasionally freeze instead of rebooting or shutting down. The i2c errors are consistent, but the freezing behavior is not.
Thank you.
drivers/i2c/i2c-core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h index 1247e6e6e975..05b8b8dfa9bd 100644 --- a/drivers/i2c/i2c-core.h +++ b/drivers/i2c/i2c-core.h @@ -29,7 +29,7 @@ int i2c_dev_irq_from_resources(const struct resource *resources, */ static inline bool i2c_in_atomic_xfer_mode(void) {
- return system_state > SYSTEM_RUNNING && irqs_disabled();
- return system_state > SYSTEM_RUNNING && !preemptible();
} static inline int __i2c_lock_bus_helper(struct i2c_adapter *adap)
-- 2.34.1
On 11/13/23 04:12, Chris Morgan wrote:
On Sat, Jul 15, 2023 at 09:53:24AM +0200, Benjamin Bara wrote:
From: Benjamin Bara benjamin.bara@skidata.com
Since bae1d3a05a8b, i2c transfers are non-atomic if preemption is disabled. However, non-atomic i2c transfers require preemption (e.g. in wait_for_completion() while waiting for the DMA).
panic() calls preempt_disable_notrace() before calling emergency_restart(). Therefore, if an i2c device is used for the restart, the xfer should be atomic. This avoids warnings like:
[ 12.667612] WARNING: CPU: 1 PID: 1 at kernel/rcu/tree_plugin.h:318 rcu_note_context_switch+0x33c/0x6b0 [ 12.676926] Voluntary context switch within RCU read-side critical section! ... [ 12.742376] schedule_timeout from wait_for_completion_timeout+0x90/0x114 [ 12.749179] wait_for_completion_timeout from tegra_i2c_wait_completion+0x40/0x70 ... [ 12.994527] atomic_notifier_call_chain from machine_restart+0x34/0x58 [ 13.001050] machine_restart from panic+0x2a8/0x32c
Use !preemptible() instead, which is basically the same check as pre-v5.2.
Fixes: bae1d3a05a8b ("i2c: core: remove use of in_atomic()") Cc: stable@vger.kernel.org # v5.2+ Suggested-by: Dmitry Osipenko dmitry.osipenko@collabora.com Acked-by: Wolfram Sang wsa@kernel.org Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com Tested-by: Nishanth Menon nm@ti.com Signed-off-by: Benjamin Bara benjamin.bara@skidata.com
For kernel 6.7 I'm having an issue when I shutdown or reboot my Rockchip RK3326 or Rockchip RK3566 based devices, and I've bisected the issue down to this specific commit.
When I shutdown or restart the device, I receive messages in the kernel log like the following:
[ 37.121148] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.122178] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.123212] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.124226] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.125242] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.126133] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x1
The device will also occasionally freeze instead of rebooting or shutting down. The i2c errors are consistent, but the freezing behavior is not.
I couldn't reproduce your issue with v6.7-rc1 and RK3399 that also uses rk3x-i2c. Though, the rk3x-i2c driver looks suspicious. Please try this patch:
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index a044ca0c35a1..aad00e9909cc 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -219,6 +219,8 @@ struct rk3x_i2c { enum rk3x_i2c_state state; unsigned int processed; int error; + + int irq; };
static inline void i2c_writel(struct rk3x_i2c *i2c, u32 value, @@ -1090,8 +1092,10 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap, rk3x_i2c_start(i2c);
if (!polling) { + enable_irq(i2c->irq); timeout = wait_event_timeout(i2c->wait, !i2c->busy, msecs_to_jiffies(WAIT_TIMEOUT)); + disable_irq(i2c->irq); } else { timeout = rk3x_i2c_wait_xfer_poll(i2c); } @@ -1236,7 +1240,6 @@ static int rk3x_i2c_probe(struct platform_device *pdev) int ret = 0; int bus_nr; u32 value; - int irq; unsigned long clk_rate;
i2c = devm_kzalloc(&pdev->dev, sizeof(struct rk3x_i2c), GFP_KERNEL); @@ -1299,11 +1302,14 @@ static int rk3x_i2c_probe(struct platform_device *pdev) }
/* IRQ setup */ - irq = platform_get_irq(pdev, 0); - if (irq < 0) - return irq; + i2c->irq = platform_get_irq(pdev, 0); + if (i2c->irq < 0) + return i2c->irq; + + /* interrupt will be enabled during of transfer time */ + irq_set_status_flags(i2c->irq, IRQ_NOAUTOEN);
- ret = devm_request_irq(&pdev->dev, irq, rk3x_i2c_irq, + ret = devm_request_irq(&pdev->dev, i2c->irq, rk3x_i2c_irq, 0, dev_name(&pdev->dev), i2c); if (ret < 0) { dev_err(&pdev->dev, "cannot request IRQ\n");
On Mon, Nov 13, 2023 at 06:46:30AM +0300, Dmitry Osipenko wrote:
On 11/13/23 04:12, Chris Morgan wrote:
On Sat, Jul 15, 2023 at 09:53:24AM +0200, Benjamin Bara wrote:
From: Benjamin Bara benjamin.bara@skidata.com
Since bae1d3a05a8b, i2c transfers are non-atomic if preemption is disabled. However, non-atomic i2c transfers require preemption (e.g. in wait_for_completion() while waiting for the DMA).
panic() calls preempt_disable_notrace() before calling emergency_restart(). Therefore, if an i2c device is used for the restart, the xfer should be atomic. This avoids warnings like:
[ 12.667612] WARNING: CPU: 1 PID: 1 at kernel/rcu/tree_plugin.h:318 rcu_note_context_switch+0x33c/0x6b0 [ 12.676926] Voluntary context switch within RCU read-side critical section! ... [ 12.742376] schedule_timeout from wait_for_completion_timeout+0x90/0x114 [ 12.749179] wait_for_completion_timeout from tegra_i2c_wait_completion+0x40/0x70 ... [ 12.994527] atomic_notifier_call_chain from machine_restart+0x34/0x58 [ 13.001050] machine_restart from panic+0x2a8/0x32c
Use !preemptible() instead, which is basically the same check as pre-v5.2.
Fixes: bae1d3a05a8b ("i2c: core: remove use of in_atomic()") Cc: stable@vger.kernel.org # v5.2+ Suggested-by: Dmitry Osipenko dmitry.osipenko@collabora.com Acked-by: Wolfram Sang wsa@kernel.org Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com Tested-by: Nishanth Menon nm@ti.com Signed-off-by: Benjamin Bara benjamin.bara@skidata.com
For kernel 6.7 I'm having an issue when I shutdown or reboot my Rockchip RK3326 or Rockchip RK3566 based devices, and I've bisected the issue down to this specific commit.
When I shutdown or restart the device, I receive messages in the kernel log like the following:
[ 37.121148] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.122178] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.123212] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.124226] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.125242] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x3 [ 37.126133] rk3x-i2c fdd40000.i2c: irq in STATE_IDLE, ipd = 0x1
The device will also occasionally freeze instead of rebooting or shutting down. The i2c errors are consistent, but the freezing behavior is not.
I couldn't reproduce your issue with v6.7-rc1 and RK3399 that also uses rk3x-i2c. Though, the rk3x-i2c driver looks suspicious. Please try this patch:
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index a044ca0c35a1..aad00e9909cc 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -219,6 +219,8 @@ struct rk3x_i2c { enum rk3x_i2c_state state; unsigned int processed; int error;
- int irq;
}; static inline void i2c_writel(struct rk3x_i2c *i2c, u32 value, @@ -1090,8 +1092,10 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap, rk3x_i2c_start(i2c); if (!polling) {
enable_irq(i2c->irq); timeout = wait_event_timeout(i2c->wait, !i2c->busy, msecs_to_jiffies(WAIT_TIMEOUT));disable_irq(i2c->irq);@@ -1236,7 +1240,6 @@ static int rk3x_i2c_probe(struct platform_device *pdev) int ret = 0; int bus_nr; u32 value;
- int irq; unsigned long clk_rate;
i2c = devm_kzalloc(&pdev->dev, sizeof(struct rk3x_i2c), GFP_KERNEL); @@ -1299,11 +1302,14 @@ static int rk3x_i2c_probe(struct platform_device *pdev) } /* IRQ setup */
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
return irq;
- i2c->irq = platform_get_irq(pdev, 0);
- if (i2c->irq < 0)
return i2c->irq;- /* interrupt will be enabled during of transfer time */
- irq_set_status_flags(i2c->irq, IRQ_NOAUTOEN);
- ret = devm_request_irq(&pdev->dev, irq, rk3x_i2c_irq,
- ret = devm_request_irq(&pdev->dev, i2c->irq, rk3x_i2c_irq, 0, dev_name(&pdev->dev), i2c); if (ret < 0) { dev_err(&pdev->dev, "cannot request IRQ\n");
I can confirm I no longer get any of the errors with this patch. Tested on both an Anbernic RG353P (RK3566 with an RK817 PMIC) and an Odroid Go Advance (RK3326 with an RK817 PMIC). The device appears to shut down consistently again and I no longer see these messages in my dmesg log when I shut down.
Thank you.
-- Best regards, Dmitry
Hi!
Thanks for testing and the feedback!
On Mon, 13 Nov 2023 at 15:54, Chris Morgan macroalpha82@gmail.com wrote:
I can confirm I no longer get any of the errors with this patch. Tested on both an Anbernic RG353P (RK3566 with an RK817 PMIC) and an Odroid Go Advance (RK3326 with an RK817 PMIC). The device appears to shut down consistently again and I no longer see these messages in my dmesg log when I shut down.
Just to make sure: Are you compiling with CONFIG_PREEMPTION (and therefore CONFIG_PREEMPT_COUNT)?
If yes, could you please also test the following patch? Because I am not sure yet how polling can be false in a "polling required" situation, meaning .master_xfer() is called instead of .master_xfer_atomic() (while your test shows that irq_disabled() is true, which is basically done with !preemptible()). The patch should test the other way round: if the situation is found, force an atomic transfer instead.
Thank you!
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index a044ca0c35a1..6e3e8433018f 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -1131,6 +1131,10 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap, static int rk3x_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) { + if (irqs_disabled()) { + WARN_ONCE(1, "Landed in non-atomic handler with disabled IRQs"); + return rk3x_i2c_xfer_common(adap, msgs, num, true); + } return rk3x_i2c_xfer_common(adap, msgs, num, false); }
On Mon, Nov 13, 2023 at 04:48:26PM +0100, Benjamin Bara wrote:
Hi!
Thanks for testing and the feedback!
On Mon, 13 Nov 2023 at 15:54, Chris Morgan macroalpha82@gmail.com wrote:
I can confirm I no longer get any of the errors with this patch. Tested on both an Anbernic RG353P (RK3566 with an RK817 PMIC) and an Odroid Go Advance (RK3326 with an RK817 PMIC). The device appears to shut down consistently again and I no longer see these messages in my dmesg log when I shut down.
Just to make sure: Are you compiling with CONFIG_PREEMPTION (and therefore CONFIG_PREEMPT_COUNT)?
If yes, could you please also test the following patch? Because I am not sure yet how polling can be false in a "polling required" situation, meaning .master_xfer() is called instead of .master_xfer_atomic() (while your test shows that irq_disabled() is true, which is basically done with !preemptible()). The patch should test the other way round: if the situation is found, force an atomic transfer instead.
Thank you!
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index a044ca0c35a1..6e3e8433018f 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -1131,6 +1131,10 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap, static int rk3x_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) {
if (irqs_disabled()) {WARN_ONCE(1, "Landed in non-atomic handler with disabled IRQs");return rk3x_i2c_xfer_common(adap, msgs, num, true);} return rk3x_i2c_xfer_common(adap, msgs, num, false);}
I have CONFIG_PREEMPT_VOLUNTARY=y but CONFIG_PREEMPTION is not set.
Thank you.
On 11/13/23 17:54, Chris Morgan wrote: ..
I can confirm I no longer get any of the errors with this patch. Tested on both an Anbernic RG353P (RK3566 with an RK817 PMIC) and an Odroid Go Advance (RK3326 with an RK817 PMIC). The device appears to shut down consistently again and I no longer see these messages in my dmesg log when I shut down.
I'll prepare the proper patch, thanks.
Hi,
Since bae1d3a05a8b, i2c transfers are non-atomic if preemption is disabled. However, non-atomic i2c transfers require preemption (e.g. in wait_for_completion() while waiting for the DMA).
panic() calls preempt_disable_notrace() before calling emergency_restart(). Therefore, if an i2c device is used for the restart, the xfer should be atomic. This avoids warnings like:
[ 12.667612] WARNING: CPU: 1 PID: 1 at kernel/rcu/tree_plugin.h:318 rcu_note_context_switch+0x33c/0x6b0 [ 12.676926] Voluntary context switch within RCU read-side critical section! ... [ 12.742376] schedule_timeout from wait_for_completion_timeout+0x90/0x114 [ 12.749179] wait_for_completion_timeout from tegra_i2c_wait_completion+0x40/0x70 ... [ 12.994527] atomic_notifier_call_chain from machine_restart+0x34/0x58 [ 13.001050] machine_restart from panic+0x2a8/0x32c
Use !preemptible() instead, which is basically the same check as pre-v5.2.
Fixes: bae1d3a05a8b ("i2c: core: remove use of in_atomic()") Cc: stable@vger.kernel.org # v5.2+ Suggested-by: Dmitry Osipenko dmitry.osipenko@collabora.com Acked-by: Wolfram Sang wsa@kernel.org Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com Tested-by: Nishanth Menon nm@ti.com Signed-off-by: Benjamin Bara benjamin.bara@skidata.com
drivers/i2c/i2c-core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h index 1247e6e6e975..05b8b8dfa9bd 100644 --- a/drivers/i2c/i2c-core.h +++ b/drivers/i2c/i2c-core.h @@ -29,7 +29,7 @@ int i2c_dev_irq_from_resources(const struct resource *resources, */ static inline bool i2c_in_atomic_xfer_mode(void) {
- return system_state > SYSTEM_RUNNING && irqs_disabled();
- return system_state > SYSTEM_RUNNING && !preemptible();
With preemption disabled, this boils down to return system_state > SYSTEM_RUNNING (&& !0)
and will then generate a backtrace splash on each reboot on our board:
# reboot -f [ 12.687169] No atomic I2C transfer handler for 'i2c-0' [ 12.692313] WARNING: CPU: 6 PID: 275 at drivers/i2c/i2c-core.h:40 i2c_smbus_xfer+0x100/0x118 [ 12.700745] Modules linked in: [ 12.703788] CPU: 6 PID: 275 Comm: reboot Not tainted 6.7.0-rc6-next-20231222+ #2494 [ 12.711431] Hardware name: Kontron 3.5"-SBC-i1200 (DT) [ 12.716555] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 12.723504] pc : i2c_smbus_xfer+0x100/0x118 [ 12.727674] lr : i2c_smbus_xfer+0x100/0x118 [ 12.731844] sp : ffff80008389b7c0 [ 12.735146] x29: ffff80008389b7c0 x28: ffff0000c561b4c0 x27: ffff0000c2b06400 [ 12.742268] x26: ffff0000c65aec4a x25: 000000000000000b x24: 0000000000000001 [ 12.749389] x23: 0000000000000000 x22: 0000000000000064 x21: 0000000000000008 [ 12.756510] x20: ffff80008389b836 x19: ffff0000c2dda080 x18: ffffffffffffffff [ 12.763631] x17: ffff800080813f48 x16: ffff80008081bd38 x15: 0730072d07630732 [ 12.770752] x14: 0769072707200772 x13: 0730072d07630732 x12: 0769072707200772 [ 12.777873] x11: 0720072007200720 x10: ffff8000827dc828 x9 : ffff80008012e154 [ 12.784994] x8 : 00000000ffffefff x7 : ffff8000827dc828 x6 : 80000000fffff000 [ 12.792116] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 [ 12.799236] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000c561b4c0 [ 12.806359] Call trace: [ 12.808793] i2c_smbus_xfer+0x100/0x118 [ 12.812616] i2c_smbus_read_i2c_block_data+0x60/0xb0 [ 12.817569] mt6360_regmap_read+0xa4/0x1b8 [ 12.821654] _regmap_raw_read+0xfc/0x270 [ 12.825566] _regmap_bus_read+0x4c/0x90 [ 12.829389] _regmap_read+0x6c/0x168 [ 12.832953] _regmap_update_bits+0xf8/0x150 [ 12.837124] regmap_update_bits_base+0x6c/0xa8 [ 12.841555] regulator_disable_regmap+0x48/0x68 [ 12.846073] _regulator_do_disable+0x130/0x1e8 [ 12.850504] _regulator_disable+0x154/0x1d8 [ 12.854676] regulator_disable+0x44/0x90 [ 12.858587] mmc_regulator_set_ocr+0x8c/0x118 [ 12.862933] msdc_ops_set_ios+0x3f4/0x938 [ 12.866932] mmc_set_initial_state+0x90/0xa8 [ 12.871191] mmc_power_off+0x40/0x68 [ 12.874754] _mmc_sd_suspend+0x5c/0x190 [ 12.878577] mmc_sd_suspend+0x20/0x78 [ 12.882227] mmc_bus_shutdown+0x48/0x90 [ 12.886051] device_shutdown+0x134/0x298 [ 12.889961] kernel_restart+0x48/0xd0 [ 12.893613] __do_sys_reboot+0x1e0/0x278 [ 12.897523] __arm64_sys_reboot+0x2c/0x40 [ 12.901519] invoke_syscall+0x50/0x128 [ 12.905257] el0_svc_common.constprop.0+0x48/0xf0 [ 12.909950] do_el0_svc+0x24/0x38 [ 12.913253] el0_svc+0x38/0xd8 [ 12.916296] el0t_64_sync_handler+0x100/0x130 [ 12.920639] el0t_64_sync+0x1a4/0x1a8 [ 12.924289] ---[ end trace 0000000000000000 ]--- ...
I'm not sure if this is now the expected behavior or not. There will be no backtraces, if I build a preemptible kernel, nor will there be backtraces if I revert this patch.
OTOH, the driver I'm using (drivers/i2c/busses/i2c-mt65xx.c) has no *_atomic(). So the warning is correct. There is also [1], which seems to be the same issue I'm facing.
-michael
[1] https://lore.kernel.org/linux-i2c/13271b9b-4132-46ef-abf8-2c311967bb46@mailb...
Hi Michael,
On Tue, 2 Jan 2024 at 16:03, Michael Walle mwalle@kernel.org wrote:
With preemption disabled, this boils down to return system_state > SYSTEM_RUNNING (&& !0)
and will then generate a backtrace splash on each reboot on our board:
# reboot -f [ 12.687169] No atomic I2C transfer handler for 'i2c-0' ... [ 12.806359] Call trace: [ 12.808793] i2c_smbus_xfer+0x100/0x118 ...
I'm not sure if this is now the expected behavior or not. There will be no backtraces, if I build a preemptible kernel, nor will there be backtraces if I revert this patch.
thanks for the report.
In your case, the warning comes from shutting down a regulator during device_shutdown(), so nothing really problematic here. However, later in the "restart sequence", IRQs are disabled before the restart handlers are called. If the reboot handlers would rely on irq-based ("non-atomic") i2c transfer, they might not work properly.
OTOH, the driver I'm using (drivers/i2c/busses/i2c-mt65xx.c) has no *_atomic(). So the warning is correct. There is also [1], which seems to be the same issue I'm facing.
-michael
[1] https://lore.kernel.org/linux-i2c/13271b9b-4132-46ef-abf8-2c311967bb46@mailb...
I tried to implement an atomic handler for the mt65xx, but I don't have the respective hardware available to test it. I decided to use a similar approach as done in drivers/i2c/busses/i2c-rk3x.c, which calls the IRQ handler in a while loop if an atomic xfer is requested. IMHO, this should work with IRQs enabled and disabled, but I am not sure if this is the best approach...
diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c index a8b5719c3372..3c18305e6059 100644 --- a/drivers/i2c/busses/i2c-mt65xx.c +++ b/drivers/i2c/busses/i2c-mt65xx.c @@ -16,6 +16,7 @@ #include <linux/interrupt.h> #include <linux/io.h> #include <linux/iopoll.h> +#include <linux/jiffies.h> #include <linux/kernel.h> #include <linux/mm.h> #include <linux/module.h> @@ -307,6 +308,8 @@ struct mtk_i2c { bool ignore_restart_irq; struct mtk_i2c_ac_timing ac_timing; const struct mtk_i2c_compatible *dev_comp; + bool atomic_xfer; + bool xfer_complete; };
/** @@ -994,6 +997,20 @@ static void i2c_dump_register(struct mtk_i2c *i2c) readl(i2c->pdmabase + OFFSET_RX_4G_MODE)); }
+static irqreturn_t mtk_i2c_irq(int irqno, void *dev_id); + +static int mtk_i2c_wait_xfer_atomic(struct mtk_i2c *i2c) +{ + unsigned long timeout = jiffies + i2c->adap.timeout; + + do { + udelay(10); + mtk_i2c_irq(0, i2c); + } while (!i2c->xfer_complete && time_before(jiffies, timeout)); + + return i2c->xfer_complete; +} + static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, int num, int left_num) { @@ -1015,7 +1032,10 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, if (i2c->auto_restart) restart_flag = I2C_RS_TRANSFER;
- reinit_completion(&i2c->msg_complete); + if (i2c->atomic_xfer) + i2c->xfer_complete = false; + else + reinit_completion(&i2c->msg_complete);
if (i2c->dev_comp->apdma_sync && i2c->op != I2C_MASTER_WRRD && num > 1) { @@ -1195,8 +1215,12 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, } mtk_i2c_writew(i2c, start_reg, OFFSET_START);
- ret = wait_for_completion_timeout(&i2c->msg_complete, - i2c->adap.timeout); + if (i2c->atomic_xfer) + /* We can't rely on wait_for_completion* calls in atomic mode. */ + ret = mtk_i2c_wait_xfer_atomic(i2c); + else + ret = wait_for_completion_timeout(&i2c->msg_complete, + i2c->adap.timeout);
/* Clear interrupt mask */ mtk_i2c_writew(i2c, ~(restart_flag | I2C_HS_NACKERR | I2C_ACKERR | @@ -1238,8 +1262,8 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs, return 0; }
-static int mtk_i2c_transfer(struct i2c_adapter *adap, - struct i2c_msg msgs[], int num) +static int mtk_i2c_transfer_common(struct i2c_adapter *adap, + struct i2c_msg msgs[], int num, bool atomic) { int ret; int left_num = num; @@ -1249,6 +1273,7 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap, if (ret) return ret;
+ i2c->atomic_xfer = atomic; i2c->auto_restart = i2c->dev_comp->auto_restart;
/* checking if we can skip restart and optimize using WRRD mode */ @@ -1303,6 +1328,18 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap, return ret; }
+static int mtk_i2c_transfer(struct i2c_adapter *adap, + struct i2c_msg msgs[], int num) +{ + return mtk_i2c_transfer_common(adap, msgs, num, false); +} + +static int mtk_i2c_transfer_atomic(struct i2c_adapter *adap, + struct i2c_msg msgs[], int num) +{ + return mtk_i2c_transfer_common(adap, msgs, num, true); +} + static irqreturn_t mtk_i2c_irq(int irqno, void *dev_id) { struct mtk_i2c *i2c = dev_id; @@ -1328,8 +1365,12 @@ static irqreturn_t mtk_i2c_irq(int irqno, void *dev_id) mtk_i2c_writew(i2c, I2C_RS_MUL_CNFG | I2C_RS_MUL_TRIG | I2C_TRANSAC_START, OFFSET_START); } else { - if (i2c->irq_stat & (I2C_TRANSAC_COMP | restart_flag)) - complete(&i2c->msg_complete); + if (i2c->irq_stat & (I2C_TRANSAC_COMP | restart_flag)) { + if (i2c->atomic_xfer) + i2c->xfer_complete = true; + else + complete(&i2c->msg_complete); + } }
return IRQ_HANDLED; @@ -1346,6 +1387,7 @@ static u32 mtk_i2c_functionality(struct i2c_adapter *adap)
static const struct i2c_algorithm mtk_i2c_algorithm = { .master_xfer = mtk_i2c_transfer, + .master_xfer_atomic = mtk_i2c_transfer_atomic, .functionality = mtk_i2c_functionality, };
Hi Benjamin,
With preemption disabled, this boils down to return system_state > SYSTEM_RUNNING (&& !0)
and will then generate a backtrace splash on each reboot on our board:
# reboot -f [ 12.687169] No atomic I2C transfer handler for 'i2c-0' ... [ 12.806359] Call trace: [ 12.808793] i2c_smbus_xfer+0x100/0x118 ...
I'm not sure if this is now the expected behavior or not. There will be no backtraces, if I build a preemptible kernel, nor will there be backtraces if I revert this patch.
thanks for the report.
In your case, the warning comes from shutting down a regulator during device_shutdown(), so nothing really problematic here.
I tend to disagree. Yes it's not problematic. But from a users point of view, you get a splash of *many* backtraces on every reboot. Btw, one should really turn this into a WARN_ONCE(). But even in this case you might scare users which will eventually lead to more bug reports.
However, later in the "restart sequence", IRQs are disabled before the restart handlers are called. If the reboot handlers would rely on irq-based ("non-atomic") i2c transfer, they might not work properly.
I get this from a technical point of view and agree that the correct fix is to add the atomic variant to the i2c driver, which begs the question, if adding the atomic variant to the driver will be considered as a Fixes patch.
Do I get it correct, that in my case the interrupts are still enabled? Otherwise I'd have gotten this warning even before your patch, correct? Excuse my ignorance, but when are the interrupts actually disabled during shutdown?
OTOH, the driver I'm using (drivers/i2c/busses/i2c-mt65xx.c) has no *_atomic(). So the warning is correct. There is also [1], which seems to be the same issue I'm facing.
-michael
[1] https://lore.kernel.org/linux-i2c/13271b9b-4132-46ef-abf8-2c311967bb46@mailb...
I tried to implement an atomic handler for the mt65xx, but I don't have the respective hardware available to test it. I decided to use a similar approach as done in drivers/i2c/busses/i2c-rk3x.c, which calls the IRQ handler in a while loop if an atomic xfer is requested. IMHO, this should work with IRQs enabled and disabled, but I am not sure if this is the best approach...
Thanks for already looking into that. Do you want to submit it as an actual patch? If so, you can add
Tested-by: Michael Walle mwalle@kernel.org
But again, it would be nice if we somehow can get rid of this huge splash of backtraces on 6.7.x (I guess it's already too late 6.7).
-michael
Hi Michael,
On Wed, 3 Jan 2024 at 10:20, Michael Walle mwalle@kernel.org wrote:
With preemption disabled, this boils down to return system_state > SYSTEM_RUNNING (&& !0)
and will then generate a backtrace splash on each reboot on our board:
# reboot -f [ 12.687169] No atomic I2C transfer handler for 'i2c-0' ... [ 12.806359] Call trace: [ 12.808793] i2c_smbus_xfer+0x100/0x118 ...
I'm not sure if this is now the expected behavior or not. There will be no backtraces, if I build a preemptible kernel, nor will there be backtraces if I revert this patch.
thanks for the report.
In your case, the warning comes from shutting down a regulator during device_shutdown(), so nothing really problematic here.
I tend to disagree. Yes it's not problematic. But from a users point of view, you get a splash of *many* backtraces on every reboot. Btw, one should really turn this into a WARN_ONCE(). But even in this case you might scare users which will eventually lead to more bug reports.
Sure, but the correct "fix" would be to implement an atomic handler if the i2c is used during this late stage. I just meant that the device_shutdown() is less problematic than the actual reboot handler. Your PMIC seems to not have a reboot handler (registered (yet)), and is therefore not "affected".
However, later in the "restart sequence", IRQs are disabled before the restart handlers are called. If the reboot handlers would rely on irq-based ("non-atomic") i2c transfer, they might not work properly.
I get this from a technical point of view and agree that the correct fix is to add the atomic variant to the i2c driver, which begs the question, if adding the atomic variant to the driver will be considered as a Fixes patch.
I can add a Fixes when I post it. Although the initial patch just makes the actual problem "noisier".
Do I get it correct, that in my case the interrupts are still enabled? Otherwise I'd have gotten this warning even before your patch, correct?
Yes, device_shutdown() is called during kernel_{shutdown,restart}_prepare(), before machine_{power_off,restart}() is called. The interrupts should therefore still be enabled in your case.
Excuse my ignorance, but when are the interrupts actually disabled during shutdown?
This is usually one of the first things done in machine_restart(), before the architecture-specific restart handlers are called (which might use i2c). Same for machine_power_off().
OTOH, the driver I'm using (drivers/i2c/busses/i2c-mt65xx.c) has no *_atomic(). So the warning is correct. There is also [1], which seems to be the same issue I'm facing.
-michael
[1] https://lore.kernel.org/linux-i2c/13271b9b-4132-46ef-abf8-2c311967bb46@mailb...
I tried to implement an atomic handler for the mt65xx, but I don't have the respective hardware available to test it. I decided to use a similar approach as done in drivers/i2c/busses/i2c-rk3x.c, which calls the IRQ handler in a while loop if an atomic xfer is requested. IMHO, this should work with IRQs enabled and disabled, but I am not sure if this is the best approach...
Thanks for already looking into that. Do you want to submit it as an actual patch? If so, you can add
Tested-by: Michael Walle mwalle@kernel.org
Yes, I can do that - thanks for the quick feedback.
But again, it would be nice if we somehow can get rid of this huge splash of backtraces on 6.7.x (I guess it's already too late 6.7).
IMHO, converting the error to WARN_ONCE() makes sense to reduce the noise, but helps having more reliable reboot handling via i2c. Do you think this is a sufficient "short-term solution" to reduce the noise before the missing atomic handlers are actually implemented?
Hi Benjamin,
With preemption disabled, this boils down to return system_state > SYSTEM_RUNNING (&& !0)
and will then generate a backtrace splash on each reboot on our board:
# reboot -f [ 12.687169] No atomic I2C transfer handler for 'i2c-0' ... [ 12.806359] Call trace: [ 12.808793] i2c_smbus_xfer+0x100/0x118 ...
I'm not sure if this is now the expected behavior or not. There will be no backtraces, if I build a preemptible kernel, nor will there be backtraces if I revert this patch.
thanks for the report.
In your case, the warning comes from shutting down a regulator during device_shutdown(), so nothing really problematic here.
I tend to disagree. Yes it's not problematic. But from a users point of view, you get a splash of *many* backtraces on every reboot. Btw, one should really turn this into a WARN_ONCE(). But even in this case you might scare users which will eventually lead to more bug reports.
Sure, but the correct "fix" would be to implement an atomic handler if the i2c is used during this late stage. I just meant that the device_shutdown() is less problematic than the actual reboot handler. Your PMIC seems to not have a reboot handler (registered (yet)), and is therefore not "affected".
However, later in the "restart sequence", IRQs are disabled before the restart handlers are called. If the reboot handlers would rely on irq-based ("non-atomic") i2c transfer, they might not work properly.
I get this from a technical point of view and agree that the correct fix is to add the atomic variant to the i2c driver, which begs the question, if adding the atomic variant to the driver will be considered as a Fixes patch.
I can add a Fixes when I post it. Although the initial patch just makes the actual problem "noisier".
As far as I understand, there was no problem (for me at least), because the interrupts were still enabled at this time. But now, there is the problem with getting these backtraces and with that the user reports.
Don't get me wrong, I'm all for the correct fix here. But at the same time I fear all the reports we'll be getting. And in the meantime there was already a new one.
Do I get it correct, that in my case the interrupts are still enabled? Otherwise I'd have gotten this warning even before your patch, correct?
Yes, device_shutdown() is called during kernel_{shutdown,restart}_prepare(), before machine_{power_off,restart}() is called. The interrupts should therefore still be enabled in your case.
Excuse my ignorance, but when are the interrupts actually disabled during shutdown?
This is usually one of the first things done in machine_restart(), before the architecture-specific restart handlers are called (which might use i2c). Same for machine_power_off().
Thanks for explaining.
OTOH, the driver I'm using (drivers/i2c/busses/i2c-mt65xx.c) has no *_atomic(). So the warning is correct. There is also [1], which seems to be the same issue I'm facing.
-michael
[1] https://lore.kernel.org/linux-i2c/13271b9b-4132-46ef-abf8-2c311967bb46@mailb...
I tried to implement an atomic handler for the mt65xx, but I don't have the respective hardware available to test it. I decided to use a similar approach as done in drivers/i2c/busses/i2c-rk3x.c, which calls the IRQ handler in a while loop if an atomic xfer is requested. IMHO, this should work with IRQs enabled and disabled, but I am not sure if this is the best approach...
Thanks for already looking into that. Do you want to submit it as an actual patch? If so, you can add
Tested-by: Michael Walle mwalle@kernel.org
Yes, I can do that - thanks for the quick feedback.
But again, it would be nice if we somehow can get rid of this huge splash of backtraces on 6.7.x (I guess it's already too late 6.7).
IMHO, converting the error to WARN_ONCE() makes sense to reduce the noise, but helps having more reliable reboot handling via i2c. Do you think this is a sufficient "short-term solution" to reduce the noise before the missing atomic handlers are actually implemented?
Turning that WARN into a WARN_ONCE is one thing. But it is still odd that don't I get a warning with preemption enabled. Is that because preemptible() will still return 1 until interrupts are actually disabled? Can we achieve something similar with kernels without preemption support? IOW, just warn iff there is an actual error, that is if i2c_xfer() is called with interrupt off?
-michael
On Sat, 15 Jul 2023 09:53:22 +0200, Benjamin Bara wrote:
The Tegra20 requires an enabled VDE power domain during startup. As the VDE is currently not used, it is disabled during runtime. Since 8f0c714ad9be, there is a workaround for the "normal restart path" which enables the VDE before doing PMC's warm reboot. This workaround is not executed in the "emergency restart path", leading to a hang-up during start.
[...]
Applied, thanks!
[1/5] kernel/reboot: emergency_restart: set correct system_state commit: 60466c067927abbcaff299845abd4b7069963139 [2/5] i2c: core: run atomic i2c xfer when !preemptible commit: aa49c90894d06e18a1ee7c095edbd2f37c232d02 [3/5] kernel/reboot: add device to sys_off_handler commit: db2d6038c5e795cab4f0a8d3e86b4f7e33338629 [4/5] mfd: tps6586x: use devm-based power off handler commit: 8bd141b17cedcbcb7d336df6e0462e4f4a528ab1 [5/5] mfd: tps6586x: register restart handler commit: 510f276df2b91efd73f6c53be62b7e692ff533c1
-- Lee Jones [李琼斯]
On Fri, 28 Jul 2023, Lee Jones wrote:
On Sat, 15 Jul 2023 09:53:22 +0200, Benjamin Bara wrote:
The Tegra20 requires an enabled VDE power domain during startup. As the VDE is currently not used, it is disabled during runtime. Since 8f0c714ad9be, there is a workaround for the "normal restart path" which enables the VDE before doing PMC's warm reboot. This workaround is not executed in the "emergency restart path", leading to a hang-up during start.
[...]
Applied, thanks!
[1/5] kernel/reboot: emergency_restart: set correct system_state commit: 60466c067927abbcaff299845abd4b7069963139 [2/5] i2c: core: run atomic i2c xfer when !preemptible commit: aa49c90894d06e18a1ee7c095edbd2f37c232d02 [3/5] kernel/reboot: add device to sys_off_handler commit: db2d6038c5e795cab4f0a8d3e86b4f7e33338629 [4/5] mfd: tps6586x: use devm-based power off handler commit: 8bd141b17cedcbcb7d336df6e0462e4f4a528ab1 [5/5] mfd: tps6586x: register restart handler commit: 510f276df2b91efd73f6c53be62b7e692ff533c1
Pull-request to follow after built tests have completed.
Hi Lee,
On Fri, 28 Jul 2023 at 12:34, Lee Jones lee@kernel.org wrote:
On Fri, 28 Jul 2023, Lee Jones wrote:
On Sat, 15 Jul 2023 09:53:22 +0200, Benjamin Bara wrote:
The Tegra20 requires an enabled VDE power domain during startup. As the VDE is currently not used, it is disabled during runtime. Since 8f0c714ad9be, there is a workaround for the "normal restart path" which enables the VDE before doing PMC's warm reboot. This workaround is not executed in the "emergency restart path", leading to a hang-up during start.
[...]
Applied, thanks!
[1/5] kernel/reboot: emergency_restart: set correct system_state commit: 60466c067927abbcaff299845abd4b7069963139 [2/5] i2c: core: run atomic i2c xfer when !preemptible commit: aa49c90894d06e18a1ee7c095edbd2f37c232d02 [3/5] kernel/reboot: add device to sys_off_handler commit: db2d6038c5e795cab4f0a8d3e86b4f7e33338629 [4/5] mfd: tps6586x: use devm-based power off handler commit: 8bd141b17cedcbcb7d336df6e0462e4f4a528ab1 [5/5] mfd: tps6586x: register restart handler commit: 510f276df2b91efd73f6c53be62b7e692ff533c1
Pull-request to follow after built tests have completed.
What's the current state of this series?
Thanks & regards Benjamin
On Thu, 07 Sep 2023, Benjamin Bara wrote:
Hi Lee,
On Fri, 28 Jul 2023 at 12:34, Lee Jones lee@kernel.org wrote:
On Fri, 28 Jul 2023, Lee Jones wrote:
On Sat, 15 Jul 2023 09:53:22 +0200, Benjamin Bara wrote:
The Tegra20 requires an enabled VDE power domain during startup. As the VDE is currently not used, it is disabled during runtime. Since 8f0c714ad9be, there is a workaround for the "normal restart path" which enables the VDE before doing PMC's warm reboot. This workaround is not executed in the "emergency restart path", leading to a hang-up during start.
[...]
Applied, thanks!
[1/5] kernel/reboot: emergency_restart: set correct system_state commit: 60466c067927abbcaff299845abd4b7069963139 [2/5] i2c: core: run atomic i2c xfer when !preemptible commit: aa49c90894d06e18a1ee7c095edbd2f37c232d02 [3/5] kernel/reboot: add device to sys_off_handler commit: db2d6038c5e795cab4f0a8d3e86b4f7e33338629 [4/5] mfd: tps6586x: use devm-based power off handler commit: 8bd141b17cedcbcb7d336df6e0462e4f4a528ab1 [5/5] mfd: tps6586x: register restart handler commit: 510f276df2b91efd73f6c53be62b7e692ff533c1
Pull-request to follow after built tests have completed.
What's the current state of this series?
Looks like the build-tests didn't complete properly, so they stayed on one of my development branches. I'll re-submit them for testing and get back to you about merging for this cycle.
Enjoy!
The following changes since commit 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5:
Linux 6.5-rc1 (2023-07-09 13:53:13 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-i2c-reboot-v6.7
for you to fetch changes up to 510f276df2b91efd73f6c53be62b7e692ff533c1:
mfd: tps6586x: Register restart handler (2023-07-28 11:33:20 +0100)
---------------------------------------------------------------- Immutable branch between MFD, I2C and Reboot due for the v6.7 merge window
---------------------------------------------------------------- Benjamin Bara (5): kernel/reboot: emergency_restart: Set correct system_state i2c: core: Run atomic i2c xfer when !preemptible kernel/reboot: Add device to sys_off_handler mfd: tps6586x: Use devm-based power off handler mfd: tps6586x: Register restart handler
drivers/i2c/i2c-core.h | 2 +- drivers/mfd/tps6586x.c | 50 ++++++++++++++++++++++++++++++++++++++++++-------- include/linux/reboot.h | 3 +++ kernel/reboot.c | 4 ++++ 4 files changed, 50 insertions(+), 9 deletions(-)
On Tue, Sep 19, 2023 at 03:46:44PM +0100, Lee Jones wrote:
Enjoy!
The following changes since commit 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5:
Linux 6.5-rc1 (2023-07-09 13:53:13 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-i2c-reboot-v6.7
for you to fetch changes up to 510f276df2b91efd73f6c53be62b7e692ff533c1:
mfd: tps6586x: Register restart handler (2023-07-28 11:33:20 +0100)
Pulled, thanks!
linux-stable-mirror@lists.linaro.org
-
Benjamin Bara -
Chris Morgan -
Dmitry Osipenko -
Lee Jones -
Michael Walle -
Wolfram Sang