diff mbox series

[v1,1/1] regulator: da9211: Use irq handler when ready

Message ID 20221124-da9211-v1-1-a54549aa6d3b@chromium.org
State New
Headers
Series regulator: da9211: Fix crash when irqs are pre-enabled |

Commit Message

Ricardo Ribalda Nov. 24, 2022, 4:45 p.m. UTC 
  If the system does not come from reset (like when it is kexec()), the
regulator might have an IRQ waiting for us.

If we enable the IRQ handler before its structures are ready, we crash.

This patch fixes:

[    1.141839] Unable to handle kernel read from unreadable memory at virtual address 0000000000000078
[    1.144475] tpm_i2c_infineon 3-0020: 1.2 TPM (device-id 0x1A)
[    1.150883] Mem abort info:
[    1.150884]   ESR = 0x96000005
[    1.150887]   EC = 0x25: DABT (current EL), IL = 32 bits
[    1.167749]   SET = 0, FnV = 0
[    1.170794]   EA = 0, S1PTW = 0
[    1.173926] Data abort info:
[    1.176798]   ISV = 0, ISS = 0x00000005
[    1.180626]   CM = 0, WnR = 0
[    1.183584] [0000000000000078] user address but active_mm is swapper
[    1.189929] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[    1.195490] Modules linked in:
[    1.198537] CPU: 2 PID: 145 Comm: irq/77-da9211 Not tainted 5.10.153-20404-gdd5053d763d6
[    1.210431] Hardware name: Google Hana (DT)
[    1.214603] pstate: 60400005 (nZCv daif +PAN -UAO -TCO BTYPE=--)
[    1.220601] pc : blocking_notifier_call_chain+0x20/0xa8
[    1.225818] lr : regulator_notifier_call_chain+0x1c/0x2c
[    1.231116] sp : ffffffc010463d40
[    1.234419] x29: ffffffc010463d40 x28: ffffff9140ad2ca4
[    1.239720] x27: ffffff914205efc0 x26: ffffff9140ad2cd8
[    1.245021] x25: 0000000000000001 x24: ffffffe9bd532b28
[    1.250321] x23: ffffff9140ad2ce0 x22: ffffff9140ad2cac
[    1.255622] x21: ffffff9140cc5a00 x20: ffffff9140ad2c00
[    1.255625] x19: ffffff9142059580 x18: fffffffffffffff5
[    1.267353] x17: 0000000000000304 x16: 00000000000003b6
[    1.267356] x15: 0000000000000002
[    1.279078] x14: 000000000018bae8
[    1.279080] x13: 0000000000100000 x12: 0000000000000239
[    1.287768] x11: 0000000000000000 x10: 0000000000000001
[    1.294202] mtk-thermal 1100b000.thermal: can't find thermal sensor 5
[    1.297582] x9 : ffffffe9bc344050 x8 : 0000000000000013
[    1.297586] x7 : 0000000000000000
[    1.309308] x6 : 0000000000000013
[    1.309310] x5 : ffffff914205ec02 x4 : 0000000000000001
[    1.309313] x3 : ffffffc010463c58 x2 : 0000000000000000
[    1.316093] x1 : 0000000000000002 x0 : 0000000000000050
[    1.316096] Call trace:
[    1.316101]  blocking_notifier_call_chain+0x20/0xa8
[    1.322757] cpu cpu0: dummy supplies not allowed for exclusive requests
[    1.327823]  regulator_notifier_call_chain+0x1c/0x2c
[    1.327825]  da9211_irq_handler+0x68/0xf8
[    1.327829]  irq_thread+0x11c/0x234
[    1.327833]  kthread+0x13c/0x154
[    1.332067] cpu cpu0: EM: created perf domain
[    1.339641]  ret_from_fork+0x10/0x30
[    1.339647] Code: a9015ff8 a90257f6 a9034ff4 910003fd (f9401408)
[    1.343215] cpu cpu2: EM: created perf domain
[    1.348335] ---[ end trace 46760cc6c7f556d8 ]---
[    1.420026] Kernel panic - not syncing: Oops: Fatal exception
[    1.425761] SMP: stopping secondary CPUs
[    1.429678] Kernel Offset: 0x29ac000000 from 0xffffffc010000000
[    1.435583] PHYS_OFFSET: 0xffffffefc0000000
[    1.439754] CPU features: 0x48240022,65806005
[    1.444099] Memory Limit: none

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Comments

Mark Brown Nov. 24, 2022, 4:53 p.m. UTC | #1 
On Thu, Nov 24, 2022 at 05:45:31PM +0100, Ricardo Ribalda wrote:

> [    1.141839] Unable to handle kernel read from unreadable memory at virtual address 0000000000000078
> [    1.144475] tpm_i2c_infineon 3-0020: 1.2 TPM (device-id 0x1A)
> [    1.150883] Mem abort info:
> [    1.150884]   ESR = 0x96000005
> [    1.150887]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    1.167749]   SET = 0, FnV = 0
> [    1.170794]   EA = 0, S1PTW = 0

Please think hard before including complete backtraces in upstream
reports, they are very large and contain almost no useful information
relative to their size so often obscure the relevant content in your
message. If part of the backtrace is usefully illustrative (it often is
for search engines if nothing else) then it's usually better to pull out
the relevant sections.
Ricardo Ribalda Nov. 24, 2022, 5:35 p.m. UTC | #2 
Hi Mark

Thanks for the review!

On Thu, 24 Nov 2022 at 17:54, Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Nov 24, 2022 at 05:45:31PM +0100, Ricardo Ribalda wrote:
>
> > [    1.141839] Unable to handle kernel read from unreadable memory at virtual address 0000000000000078
> > [    1.144475] tpm_i2c_infineon 3-0020: 1.2 TPM (device-id 0x1A)
> > [    1.150883] Mem abort info:
> > [    1.150884]   ESR = 0x96000005
> > [    1.150887]   EC = 0x25: DABT (current EL), IL = 32 bits
> > [    1.167749]   SET = 0, FnV = 0
> > [    1.170794]   EA = 0, S1PTW = 0
>
> Please think hard before including complete backtraces in upstream
> reports, they are very large and contain almost no useful information
> relative to their size so often obscure the relevant content in your
> message. If part of the backtrace is usefully illustrative (it often is
> for search engines if nothing else) then it's usually better to pull out
> the relevant sections.

Do you want that I post a v2 of the patch?

Regards
Mark Brown Nov. 24, 2022, 5:41 p.m. UTC | #3 
On Thu, Nov 24, 2022 at 06:35:42PM +0100, Ricardo Ribalda wrote:

> On Thu, 24 Nov 2022 at 17:54, Mark Brown <broonie@kernel.org> wrote:
> > On Thu, Nov 24, 2022 at 05:45:31PM +0100, Ricardo Ribalda wrote:

> > Please think hard before including complete backtraces in upstream
> > reports, they are very large and contain almost no useful information
> > relative to their size so often obscure the relevant content in your
> > message. If part of the backtrace is usefully illustrative (it often is
> > for search engines if nothing else) then it's usually better to pull out
> > the relevant sections.

> Do you want that I post a v2 of the patch?

It's fine, just please think about this for future submissions.
DLG Adam Ward Nov. 27, 2022, 8:55 p.m. UTC | #4 
On 24 November 2022 16:46, Ricardo Ribalda wrote:
>If the system does not come from reset (like when it is kexec()), the regulator might have an IRQ waiting for us.
>
>If we enable the IRQ handler before its structures are ready, we crash.

A good catch, thank you :-)

...
>+	ret = da9211_regulator_init(chip);
...
> 	chip->chip_irq = i2c->irq;

On the other hand... with the init call made before chip_irq is assigned, the IRQs won't be unmasked:
	https://elixir.bootlin.com/linux/latest/source/drivers/regulator/da9211-regulator.c#L430

Nacked-by: Adam Ward <DLG-Adam.Ward.opensource@dm.renesas.com>

Regards,
Adam
diff mbox series

Patch

diff --git a/drivers/regulator/da9211-regulator.c b/drivers/regulator/da9211-regulator.c
index e01b32d1fa17..36a0009e23de 100644
--- a/drivers/regulator/da9211-regulator.c
+++ b/drivers/regulator/da9211-regulator.c
@@ -496,6 +496,12 @@  static int da9211_i2c_probe(struct i2c_client *i2c)
 		return PTR_ERR(chip->pdata);
 	}
 
+	ret = da9211_regulator_init(chip);
+	if (ret < 0) {
+		dev_err(chip->dev, "Failed to initialize regulator: %d\n", ret);
+		return ret;
+	}
+
 	chip->chip_irq = i2c->irq;
 
 	if (chip->chip_irq != 0) {
@@ -512,11 +518,6 @@  static int da9211_i2c_probe(struct i2c_client *i2c)
 		dev_warn(chip->dev, "No IRQ configured\n");
 	}
 
-	ret = da9211_regulator_init(chip);
-
-	if (ret < 0)
-		dev_err(chip->dev, "Failed to initialize regulator: %d\n", ret);
-
 	return ret;
 }