| Message ID | 20220711130522.401551-1-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found | expand |
On Mon, Jul 11, 2022 at 02:05:22PM +0100, Alexandru Elisei wrote: > Commit a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") switched > BCLK to GPIO functions when probing the i2s bus interface, but missed > adding a check for when devm_pinctrl_get() returns an error. This can lead > to the following NULL pointer dereference on a rockpro64-v2 if there are no > "pinctrl" properties in the i2s device tree node: > > [ 0.658381] rockchip-i2s ff880000.i2s: failed to find i2s default state > [ 0.658993] rockchip-i2s ff880000.i2s: failed to find i2s gpio state > [ 0.660072] rockchip-i2s ff890000.i2s: failed to find i2s default state > [ 0.660670] rockchip-i2s ff890000.i2s: failed to find i2s gpio state 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.
Hi Mark, On Mon, Jul 11, 2022 at 02:23:59PM +0100, Mark Brown wrote: > On Mon, Jul 11, 2022 at 02:05:22PM +0100, Alexandru Elisei wrote: > > Commit a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") switched > > BCLK to GPIO functions when probing the i2s bus interface, but missed > > adding a check for when devm_pinctrl_get() returns an error. This can lead > > to the following NULL pointer dereference on a rockpro64-v2 if there are no > > "pinctrl" properties in the i2s device tree node: > > > > [ 0.658381] rockchip-i2s ff880000.i2s: failed to find i2s default state > > [ 0.658993] rockchip-i2s ff880000.i2s: failed to find i2s gpio state > > [ 0.660072] rockchip-i2s ff890000.i2s: failed to find i2s default state > > [ 0.660670] rockchip-i2s ff890000.i2s: failed to find i2s gpio state > > Please think hard before including complete backtraces in upstream > reports, they are very large and contain almost no useful information I'm at a loss here. Are you saying that those 4 lines represent a complete backtrace and they are very large? Or are you talking about the panic log that I've included in the commit message? > 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. Would you mind pointing out what you think the relevant sections are? I would also find it very useful (for future patches) if you can explain why they are relevant, and why those parts you've left out aren't. It's not very easy to figure out what is relevant when you're not familiar with a subsystem. Thanks, Alex
On Mon, Jul 11, 2022 at 02:40:08PM +0100, Alexandru Elisei wrote: > On Mon, Jul 11, 2022 at 02:23:59PM +0100, Mark Brown wrote: > > Please think hard before including complete backtraces in upstream > > reports, they are very large and contain almost no useful information > I'm at a loss here. Are you saying that those 4 lines represent a complete > backtrace and they are very large? Or are you talking about the panic log > that I've included in the commit message? I'm talking about the entire log that that was the start of, I deleted the bulk of it due to the excessive size. > > 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. > Would you mind pointing out what you think the relevant sections are? I > would also find it very useful (for future patches) if you can explain why > they are relevant, and why those parts you've left out aren't. It's not > very easy to figure out what is relevant when you're not familiar with a > subsystem. It really depends what the information you're trying to convey with the backtrace is, in general a couple of frames of context might be useful if there's something interesting about the context from which things were called since that's the unique bit that people might search for. For example things like the standard set of generic functions you'd see when probing a device is rarely going to convey anything meaningful, and similarly the standard kernel entry backtrace for something triggered from a system call. The full register state is also commonly not of any great relevance if it's not illustrating something in the rest of the message. If you are just including an entire splat on the off chance that it might be relevant consider just not including it rather than including everything.
On Mon, 11 Jul 2022 14:05:22 +0100, Alexandru Elisei wrote: > Commit a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") switched > BCLK to GPIO functions when probing the i2s bus interface, but missed > adding a check for when devm_pinctrl_get() returns an error. This can lead > to the following NULL pointer dereference on a rockpro64-v2 if there are no > "pinctrl" properties in the i2s device tree node: > > [ 0.658381] rockchip-i2s ff880000.i2s: failed to find i2s default state > [ 0.658993] rockchip-i2s ff880000.i2s: failed to find i2s gpio state > [ 0.660072] rockchip-i2s ff890000.i2s: failed to find i2s default state > [ 0.660670] rockchip-i2s ff890000.i2s: failed to find i2s gpio state > [ 0.661716] rockchip-i2s ff8a0000.i2s: failed to find i2s pinctrl > [ 0.662276] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000005 > [ 0.663061] Mem abort info: > [ 0.663317] ESR = 0x0000000096000004 > [ 0.663658] EC = 0x25: DABT (current EL), IL = 32 bits > [ 0.664136] SET = 0, FnV = 0 > [ 0.664171] mmc2: SDHCI controller on fe330000.mmc [fe330000.mmc] using ADMA > [ 0.664409] EA = 0, S1PTW = 0 > [ 0.664415] FSC = 0x04: level 0 translation fault > [ 0.664421] Data abort info: > [ 0.666050] ISV = 0, ISS = 0x00000004 > [ 0.666399] CM = 0, WnR = 0 > [ 0.666671] [0000000000000005] user address but active_mm is swapper > [ 0.667240] Internal error: Oops: 96000004 [#1] PREEMPT SMP > [ 0.667742] Modules linked in: > [ 0.668028] CPU: 5 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc6 #300 > [ 0.668608] Hardware name: Pine64 RockPro64 v2.0 (DT) > [ 0.669062] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 0.669689] pc : pinctrl_lookup_state+0x20/0xc0 > [ 0.670110] lr : rockchip_i2s_probe+0x1a8/0x54c > [ 0.670527] sp : ffff80000a17bb30 > [ 0.670829] x29: ffff80000a17bb30 x28: 0000000000000000 x27: ffff8000097c04c8 > [ 0.671480] x26: ffff800009871060 x25: ffff800009871078 x24: ffff000001c11368 > [ 0.672129] x23: ffff8000092dc850 x22: ffffffffffffffed x21: ffff8000096f7e98 > [ 0.672776] x20: ffffffffffffffed x19: ffff000001d92480 x18: ffffffffffffffff > [ 0.673423] x17: 000000040044ffff x16: ffff0000f77db2d0 x15: 0764076e07690766 > [ 0.674070] x14: 0720076f07740720 x13: ffff800009e129f0 x12: 000000000000038d > [ 0.674717] x11: 000000000000012f x10: ffff800009e6a9f0 x9 : ffff800009e129f0 > [ 0.675364] x8 : 00000000ffffefff x7 : ffff800009e6a9f0 x6 : 80000000fffff000 > [ 0.676011] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 > [ 0.676657] x2 : 0000000000000000 x1 : ffff8000096f7e98 x0 : ffffffffffffffed > [ 0.677304] Call trace: > [ 0.677531] pinctrl_lookup_state+0x20/0xc0 > [ 0.677914] rockchip_i2s_probe+0x1a8/0x54c > [ 0.678297] platform_probe+0x68/0xc0 > [ 0.678638] really_probe.part.0+0x9c/0x2ac > [ 0.679027] __driver_probe_device+0x98/0x144 > [ 0.679429] driver_probe_device+0xac/0x140 > [ 0.679814] __driver_attach+0xf8/0x184 > [ 0.680169] bus_for_each_dev+0x70/0xd0 > [ 0.680524] driver_attach+0x24/0x30 > [ 0.680856] bus_add_driver+0x150/0x200 > [ 0.681210] driver_register+0x78/0x130 > [ 0.681560] __platform_driver_register+0x28/0x34 > [ 0.681988] rockchip_i2s_driver_init+0x1c/0x28 > [ 0.682407] do_one_initcall+0x50/0x1c0 > [ 0.682760] kernel_init_freeable+0x204/0x288 > [ 0.683160] kernel_init+0x28/0x13c > [ 0.683482] ret_from_fork+0x10/0x20 > [ 0.683816] Code: aa0003f4 a9025bf5 aa0003f6 aa0103f5 (f8418e93) > [ 0.684365] ---[ end trace 0000000000000000 ]--- > [ 0.684813] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b > [ 0.685500] SMP: stopping secondary CPUs > [ 0.685995] Kernel Offset: disabled > [ 0.686310] CPU features: 0x800,00105811,00001086 > [ 0.686736] Memory Limit: none > [ 0.687021] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]--- > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found commit: 26b9f2fa7b1c6aba6fa9b83274a3e54868f69562 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Hi, On Mon, Jul 11, 2022 at 03:41:31PM +0100, Mark Brown wrote: > On Mon, Jul 11, 2022 at 02:40:08PM +0100, Alexandru Elisei wrote: > > On Mon, Jul 11, 2022 at 02:23:59PM +0100, Mark Brown wrote: > > > > Please think hard before including complete backtraces in upstream > > > reports, they are very large and contain almost no useful information > > > I'm at a loss here. Are you saying that those 4 lines represent a complete > > backtrace and they are very large? Or are you talking about the panic log > > that I've included in the commit message? > > I'm talking about the entire log that that was the start of, I deleted > the bulk of it due to the excessive size. Oh, that makes sense then. > > > > 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. > > > Would you mind pointing out what you think the relevant sections are? I > > would also find it very useful (for future patches) if you can explain why > > they are relevant, and why those parts you've left out aren't. It's not > > very easy to figure out what is relevant when you're not familiar with a > > subsystem. > > It really depends what the information you're trying to convey with the > backtrace is, in general a couple of frames of context might be useful > if there's something interesting about the context from which things > were called since that's the unique bit that people might search for. > For example things like the standard set of generic functions you'd see > when probing a device is rarely going to convey anything meaningful, and > similarly the standard kernel entry backtrace for something triggered > from a system call. The full register state is also commonly not of any > great relevance if it's not illustrating something in the rest of the > message. > > If you are just including an entire splat on the off chance that it > might be relevant consider just not including it rather than including > everything. Thanks for the explanation! I've included the entire splat, not just the bits that prove my conclusion, in case my conclusion is wrong and someone reading the backtrace can notice what I missed. In this case, the splat was definitely useful for me, because the last two functions from the call trace point to where the bug was. Do you want me to respin the patch with an abbreviated splat? Thanks, Alex
On Mon, Jul 11, 2022 at 04:17:37PM +0100, Alexandru Elisei wrote:
> Do you want me to respin the patch with an abbreviated splat?
It's fine, just leave this one.
On Mon, Jul 11, 2022 at 9:06 PM Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > Commit a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") switched > BCLK to GPIO functions when probing the i2s bus interface, but missed > adding a check for when devm_pinctrl_get() returns an error. This can lead > to the following NULL pointer dereference on a rockpro64-v2 if there are no > "pinctrl" properties in the i2s device tree node: > > [ 0.658381] rockchip-i2s ff880000.i2s: failed to find i2s default state > [ 0.658993] rockchip-i2s ff880000.i2s: failed to find i2s gpio state > [ 0.660072] rockchip-i2s ff890000.i2s: failed to find i2s default state > [ 0.660670] rockchip-i2s ff890000.i2s: failed to find i2s gpio state > [ 0.661716] rockchip-i2s ff8a0000.i2s: failed to find i2s pinctrl > [ 0.662276] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000005 > [ 0.663061] Mem abort info: > [ 0.663317] ESR = 0x0000000096000004 > [ 0.663658] EC = 0x25: DABT (current EL), IL = 32 bits > [ 0.664136] SET = 0, FnV = 0 > [ 0.664171] mmc2: SDHCI controller on fe330000.mmc [fe330000.mmc] using ADMA > [ 0.664409] EA = 0, S1PTW = 0 > [ 0.664415] FSC = 0x04: level 0 translation fault > [ 0.664421] Data abort info: > [ 0.666050] ISV = 0, ISS = 0x00000004 > [ 0.666399] CM = 0, WnR = 0 > [ 0.666671] [0000000000000005] user address but active_mm is swapper > [ 0.667240] Internal error: Oops: 96000004 [#1] PREEMPT SMP > [ 0.667742] Modules linked in: > [ 0.668028] CPU: 5 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc6 #300 > [ 0.668608] Hardware name: Pine64 RockPro64 v2.0 (DT) > [ 0.669062] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 0.669689] pc : pinctrl_lookup_state+0x20/0xc0 > [ 0.670110] lr : rockchip_i2s_probe+0x1a8/0x54c > [ 0.670527] sp : ffff80000a17bb30 > [ 0.670829] x29: ffff80000a17bb30 x28: 0000000000000000 x27: ffff8000097c04c8 > [ 0.671480] x26: ffff800009871060 x25: ffff800009871078 x24: ffff000001c11368 > [ 0.672129] x23: ffff8000092dc850 x22: ffffffffffffffed x21: ffff8000096f7e98 > [ 0.672776] x20: ffffffffffffffed x19: ffff000001d92480 x18: ffffffffffffffff > [ 0.673423] x17: 000000040044ffff x16: ffff0000f77db2d0 x15: 0764076e07690766 > [ 0.674070] x14: 0720076f07740720 x13: ffff800009e129f0 x12: 000000000000038d > [ 0.674717] x11: 000000000000012f x10: ffff800009e6a9f0 x9 : ffff800009e129f0 > [ 0.675364] x8 : 00000000ffffefff x7 : ffff800009e6a9f0 x6 : 80000000fffff000 > [ 0.676011] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 > [ 0.676657] x2 : 0000000000000000 x1 : ffff8000096f7e98 x0 : ffffffffffffffed > [ 0.677304] Call trace: > [ 0.677531] pinctrl_lookup_state+0x20/0xc0 > [ 0.677914] rockchip_i2s_probe+0x1a8/0x54c > [ 0.678297] platform_probe+0x68/0xc0 > [ 0.678638] really_probe.part.0+0x9c/0x2ac > [ 0.679027] __driver_probe_device+0x98/0x144 > [ 0.679429] driver_probe_device+0xac/0x140 > [ 0.679814] __driver_attach+0xf8/0x184 > [ 0.680169] bus_for_each_dev+0x70/0xd0 > [ 0.680524] driver_attach+0x24/0x30 > [ 0.680856] bus_add_driver+0x150/0x200 > [ 0.681210] driver_register+0x78/0x130 > [ 0.681560] __platform_driver_register+0x28/0x34 > [ 0.681988] rockchip_i2s_driver_init+0x1c/0x28 > [ 0.682407] do_one_initcall+0x50/0x1c0 > [ 0.682760] kernel_init_freeable+0x204/0x288 > [ 0.683160] kernel_init+0x28/0x13c > [ 0.683482] ret_from_fork+0x10/0x20 > [ 0.683816] Code: aa0003f4 a9025bf5 aa0003f6 aa0103f5 (f8418e93) > [ 0.684365] ---[ end trace 0000000000000000 ]--- > [ 0.684813] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b > [ 0.685500] SMP: stopping secondary CPUs > [ 0.685995] Kernel Offset: disabled > [ 0.686310] CPU features: 0x800,00105811,00001086 > [ 0.686736] Memory Limit: none > [ 0.687021] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]--- > > Check that i2s->pinctrl is valid before attempting to search for the > bclk_on and bclk_off pinctrl states. > > Fixes: a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- > > Full log at [1], config at [2] (pastebins expire after 6 months). > > I'm not familiar with this part of the kernel, I did my best to come up > with an explanation and a fix for the panic. > > Read Documentation/devicetree/bindings/sound/rockchip-i2s.yaml, which has the > definition for the i2s nodes with the same compatible string as the i2s@ff8a0000 > that is causing the panic (which is, "rockchip,rk3399-i2s" > "rockchip,rk3066-i2s"). There's no mention there of a "pinctrl" property, maybe > I'm reading the docs wrong, or maybe the board devicetree also needs fixing. > > [1] https://pastebin.com/vuRVDsKk > [2] https://pastebin.com/3yDMF7YE > > sound/soc/rockchip/rockchip_i2s.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c > index 99a128a666fb..c9fedf6eb2e6 100644 > --- a/sound/soc/rockchip/rockchip_i2s.c > +++ b/sound/soc/rockchip/rockchip_i2s.c > @@ -808,8 +808,11 @@ static int rockchip_i2s_probe(struct platform_device *pdev) > > i2s->bclk_ratio = 64; > i2s->pinctrl = devm_pinctrl_get(&pdev->dev); > - if (IS_ERR(i2s->pinctrl)) > + if (IS_ERR(i2s->pinctrl)) { > dev_err(&pdev->dev, "failed to find i2s pinctrl\n"); > + ret = PTR_ERR(i2s->pinctrl); > + goto err_clk; > + } This would break audio for HDMI audio, which uses an I2S interface to feed the HDMI controller and does not need or have pinctrl, but here you make pinctrl a requirement. See https://lore.kernel.org/alsa-devel/20220621185747.2782-1-wens@kernel.org/ for my fix, which is merged for 5.20. Maybe your patch (which Mark already applied) and commit a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") should just be reverted from the for-5.19? Mark? Regards ChenYu > > i2s->bclk_on = pinctrl_lookup_state(i2s->pinctrl, > "bclk_on"); > -- > 2.37.0 > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hi ChenYu, On Tue, Jul 12, 2022 at 02:17:32PM +0800, Chen-Yu Tsai wrote: > On Mon, Jul 11, 2022 at 9:06 PM Alexandru Elisei > <alexandru.elisei@arm.com> wrote: > > > > Commit a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") switched > > BCLK to GPIO functions when probing the i2s bus interface, but missed > > adding a check for when devm_pinctrl_get() returns an error. This can lead > > to the following NULL pointer dereference on a rockpro64-v2 if there are no > > "pinctrl" properties in the i2s device tree node: > > > > [ 0.658381] rockchip-i2s ff880000.i2s: failed to find i2s default state > > [ 0.658993] rockchip-i2s ff880000.i2s: failed to find i2s gpio state > > [ 0.660072] rockchip-i2s ff890000.i2s: failed to find i2s default state > > [ 0.660670] rockchip-i2s ff890000.i2s: failed to find i2s gpio state > > [ 0.661716] rockchip-i2s ff8a0000.i2s: failed to find i2s pinctrl > > [ 0.662276] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000005 > > [ 0.663061] Mem abort info: > > [ 0.663317] ESR = 0x0000000096000004 > > [ 0.663658] EC = 0x25: DABT (current EL), IL = 32 bits > > [ 0.664136] SET = 0, FnV = 0 > > [ 0.664171] mmc2: SDHCI controller on fe330000.mmc [fe330000.mmc] using ADMA > > [ 0.664409] EA = 0, S1PTW = 0 > > [ 0.664415] FSC = 0x04: level 0 translation fault > > [ 0.664421] Data abort info: > > [ 0.666050] ISV = 0, ISS = 0x00000004 > > [ 0.666399] CM = 0, WnR = 0 > > [ 0.666671] [0000000000000005] user address but active_mm is swapper > > [ 0.667240] Internal error: Oops: 96000004 [#1] PREEMPT SMP > > [ 0.667742] Modules linked in: > > [ 0.668028] CPU: 5 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc6 #300 > > [ 0.668608] Hardware name: Pine64 RockPro64 v2.0 (DT) > > [ 0.669062] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > [ 0.669689] pc : pinctrl_lookup_state+0x20/0xc0 > > [ 0.670110] lr : rockchip_i2s_probe+0x1a8/0x54c > > [ 0.670527] sp : ffff80000a17bb30 > > [ 0.670829] x29: ffff80000a17bb30 x28: 0000000000000000 x27: ffff8000097c04c8 > > [ 0.671480] x26: ffff800009871060 x25: ffff800009871078 x24: ffff000001c11368 > > [ 0.672129] x23: ffff8000092dc850 x22: ffffffffffffffed x21: ffff8000096f7e98 > > [ 0.672776] x20: ffffffffffffffed x19: ffff000001d92480 x18: ffffffffffffffff > > [ 0.673423] x17: 000000040044ffff x16: ffff0000f77db2d0 x15: 0764076e07690766 > > [ 0.674070] x14: 0720076f07740720 x13: ffff800009e129f0 x12: 000000000000038d > > [ 0.674717] x11: 000000000000012f x10: ffff800009e6a9f0 x9 : ffff800009e129f0 > > [ 0.675364] x8 : 00000000ffffefff x7 : ffff800009e6a9f0 x6 : 80000000fffff000 > > [ 0.676011] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 > > [ 0.676657] x2 : 0000000000000000 x1 : ffff8000096f7e98 x0 : ffffffffffffffed > > [ 0.677304] Call trace: > > [ 0.677531] pinctrl_lookup_state+0x20/0xc0 > > [ 0.677914] rockchip_i2s_probe+0x1a8/0x54c > > [ 0.678297] platform_probe+0x68/0xc0 > > [ 0.678638] really_probe.part.0+0x9c/0x2ac > > [ 0.679027] __driver_probe_device+0x98/0x144 > > [ 0.679429] driver_probe_device+0xac/0x140 > > [ 0.679814] __driver_attach+0xf8/0x184 > > [ 0.680169] bus_for_each_dev+0x70/0xd0 > > [ 0.680524] driver_attach+0x24/0x30 > > [ 0.680856] bus_add_driver+0x150/0x200 > > [ 0.681210] driver_register+0x78/0x130 > > [ 0.681560] __platform_driver_register+0x28/0x34 > > [ 0.681988] rockchip_i2s_driver_init+0x1c/0x28 > > [ 0.682407] do_one_initcall+0x50/0x1c0 > > [ 0.682760] kernel_init_freeable+0x204/0x288 > > [ 0.683160] kernel_init+0x28/0x13c > > [ 0.683482] ret_from_fork+0x10/0x20 > > [ 0.683816] Code: aa0003f4 a9025bf5 aa0003f6 aa0103f5 (f8418e93) > > [ 0.684365] ---[ end trace 0000000000000000 ]--- > > [ 0.684813] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b > > [ 0.685500] SMP: stopping secondary CPUs > > [ 0.685995] Kernel Offset: disabled > > [ 0.686310] CPU features: 0x800,00105811,00001086 > > [ 0.686736] Memory Limit: none > > [ 0.687021] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]--- > > > > Check that i2s->pinctrl is valid before attempting to search for the > > bclk_on and bclk_off pinctrl states. > > > > Fixes: a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > > --- > > > > Full log at [1], config at [2] (pastebins expire after 6 months). > > > > I'm not familiar with this part of the kernel, I did my best to come up > > with an explanation and a fix for the panic. > > > > Read Documentation/devicetree/bindings/sound/rockchip-i2s.yaml, which has the > > definition for the i2s nodes with the same compatible string as the i2s@ff8a0000 > > that is causing the panic (which is, "rockchip,rk3399-i2s" > > "rockchip,rk3066-i2s"). There's no mention there of a "pinctrl" property, maybe > > I'm reading the docs wrong, or maybe the board devicetree also needs fixing. > > > > [1] https://pastebin.com/vuRVDsKk > > [2] https://pastebin.com/3yDMF7YE > > > > sound/soc/rockchip/rockchip_i2s.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c > > index 99a128a666fb..c9fedf6eb2e6 100644 > > --- a/sound/soc/rockchip/rockchip_i2s.c > > +++ b/sound/soc/rockchip/rockchip_i2s.c > > @@ -808,8 +808,11 @@ static int rockchip_i2s_probe(struct platform_device *pdev) > > > > i2s->bclk_ratio = 64; > > i2s->pinctrl = devm_pinctrl_get(&pdev->dev); > > - if (IS_ERR(i2s->pinctrl)) > > + if (IS_ERR(i2s->pinctrl)) { > > dev_err(&pdev->dev, "failed to find i2s pinctrl\n"); > > + ret = PTR_ERR(i2s->pinctrl); > > + goto err_clk; > > + } > > This would break audio for HDMI audio, which uses an I2S interface > to feed the HDMI controller and does not need or have pinctrl, but > here you make pinctrl a requirement. > > See https://lore.kernel.org/alsa-devel/20220621185747.2782-1-wens@kernel.org/ > for my fix, which is merged for 5.20. For what it's worth, I've tested the fix and the board boots just fine. Thanks, Alex > > Maybe your patch (which Mark already applied) and commit a5450aba737d > ("ASoC: rockchip: i2s: switch BCLK to GPIO") should just be reverted from > the for-5.19? > > Mark? > > > Regards > ChenYu > > > > > i2s->bclk_on = pinctrl_lookup_state(i2s->pinctrl, > > "bclk_on"); > > -- > > 2.37.0 > > > > > > _______________________________________________ > > Linux-rockchip mailing list > > Linux-rockchip@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
On Tue, Jul 12, 2022 at 4:53 PM Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > Hi ChenYu, > > On Tue, Jul 12, 2022 at 02:17:32PM +0800, Chen-Yu Tsai wrote: > > On Mon, Jul 11, 2022 at 9:06 PM Alexandru Elisei > > <alexandru.elisei@arm.com> wrote: > > > > > > Commit a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") switched > > > BCLK to GPIO functions when probing the i2s bus interface, but missed > > > adding a check for when devm_pinctrl_get() returns an error. This can lead > > > to the following NULL pointer dereference on a rockpro64-v2 if there are no > > > "pinctrl" properties in the i2s device tree node: > > > > > > [ 0.658381] rockchip-i2s ff880000.i2s: failed to find i2s default state > > > [ 0.658993] rockchip-i2s ff880000.i2s: failed to find i2s gpio state > > > [ 0.660072] rockchip-i2s ff890000.i2s: failed to find i2s default state > > > [ 0.660670] rockchip-i2s ff890000.i2s: failed to find i2s gpio state > > > [ 0.661716] rockchip-i2s ff8a0000.i2s: failed to find i2s pinctrl > > > [ 0.662276] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000005 > > > [ 0.663061] Mem abort info: > > > [ 0.663317] ESR = 0x0000000096000004 > > > [ 0.663658] EC = 0x25: DABT (current EL), IL = 32 bits > > > [ 0.664136] SET = 0, FnV = 0 > > > [ 0.664171] mmc2: SDHCI controller on fe330000.mmc [fe330000.mmc] using ADMA > > > [ 0.664409] EA = 0, S1PTW = 0 > > > [ 0.664415] FSC = 0x04: level 0 translation fault > > > [ 0.664421] Data abort info: > > > [ 0.666050] ISV = 0, ISS = 0x00000004 > > > [ 0.666399] CM = 0, WnR = 0 > > > [ 0.666671] [0000000000000005] user address but active_mm is swapper > > > [ 0.667240] Internal error: Oops: 96000004 [#1] PREEMPT SMP > > > [ 0.667742] Modules linked in: > > > [ 0.668028] CPU: 5 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc6 #300 > > > [ 0.668608] Hardware name: Pine64 RockPro64 v2.0 (DT) > > > [ 0.669062] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > > [ 0.669689] pc : pinctrl_lookup_state+0x20/0xc0 > > > [ 0.670110] lr : rockchip_i2s_probe+0x1a8/0x54c > > > [ 0.670527] sp : ffff80000a17bb30 > > > [ 0.670829] x29: ffff80000a17bb30 x28: 0000000000000000 x27: ffff8000097c04c8 > > > [ 0.671480] x26: ffff800009871060 x25: ffff800009871078 x24: ffff000001c11368 > > > [ 0.672129] x23: ffff8000092dc850 x22: ffffffffffffffed x21: ffff8000096f7e98 > > > [ 0.672776] x20: ffffffffffffffed x19: ffff000001d92480 x18: ffffffffffffffff > > > [ 0.673423] x17: 000000040044ffff x16: ffff0000f77db2d0 x15: 0764076e07690766 > > > [ 0.674070] x14: 0720076f07740720 x13: ffff800009e129f0 x12: 000000000000038d > > > [ 0.674717] x11: 000000000000012f x10: ffff800009e6a9f0 x9 : ffff800009e129f0 > > > [ 0.675364] x8 : 00000000ffffefff x7 : ffff800009e6a9f0 x6 : 80000000fffff000 > > > [ 0.676011] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 > > > [ 0.676657] x2 : 0000000000000000 x1 : ffff8000096f7e98 x0 : ffffffffffffffed > > > [ 0.677304] Call trace: > > > [ 0.677531] pinctrl_lookup_state+0x20/0xc0 > > > [ 0.677914] rockchip_i2s_probe+0x1a8/0x54c > > > [ 0.678297] platform_probe+0x68/0xc0 > > > [ 0.678638] really_probe.part.0+0x9c/0x2ac > > > [ 0.679027] __driver_probe_device+0x98/0x144 > > > [ 0.679429] driver_probe_device+0xac/0x140 > > > [ 0.679814] __driver_attach+0xf8/0x184 > > > [ 0.680169] bus_for_each_dev+0x70/0xd0 > > > [ 0.680524] driver_attach+0x24/0x30 > > > [ 0.680856] bus_add_driver+0x150/0x200 > > > [ 0.681210] driver_register+0x78/0x130 > > > [ 0.681560] __platform_driver_register+0x28/0x34 > > > [ 0.681988] rockchip_i2s_driver_init+0x1c/0x28 > > > [ 0.682407] do_one_initcall+0x50/0x1c0 > > > [ 0.682760] kernel_init_freeable+0x204/0x288 > > > [ 0.683160] kernel_init+0x28/0x13c > > > [ 0.683482] ret_from_fork+0x10/0x20 > > > [ 0.683816] Code: aa0003f4 a9025bf5 aa0003f6 aa0103f5 (f8418e93) > > > [ 0.684365] ---[ end trace 0000000000000000 ]--- > > > [ 0.684813] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b > > > [ 0.685500] SMP: stopping secondary CPUs > > > [ 0.685995] Kernel Offset: disabled > > > [ 0.686310] CPU features: 0x800,00105811,00001086 > > > [ 0.686736] Memory Limit: none > > > [ 0.687021] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]--- > > > > > > Check that i2s->pinctrl is valid before attempting to search for the > > > bclk_on and bclk_off pinctrl states. > > > > > > Fixes: a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > > > --- > > > > > > Full log at [1], config at [2] (pastebins expire after 6 months). > > > > > > I'm not familiar with this part of the kernel, I did my best to come up > > > with an explanation and a fix for the panic. > > > > > > Read Documentation/devicetree/bindings/sound/rockchip-i2s.yaml, which has the > > > definition for the i2s nodes with the same compatible string as the i2s@ff8a0000 > > > that is causing the panic (which is, "rockchip,rk3399-i2s" > > > "rockchip,rk3066-i2s"). There's no mention there of a "pinctrl" property, maybe > > > I'm reading the docs wrong, or maybe the board devicetree also needs fixing. > > > > > > [1] https://pastebin.com/vuRVDsKk > > > [2] https://pastebin.com/3yDMF7YE > > > > > > sound/soc/rockchip/rockchip_i2s.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c > > > index 99a128a666fb..c9fedf6eb2e6 100644 > > > --- a/sound/soc/rockchip/rockchip_i2s.c > > > +++ b/sound/soc/rockchip/rockchip_i2s.c > > > @@ -808,8 +808,11 @@ static int rockchip_i2s_probe(struct platform_device *pdev) > > > > > > i2s->bclk_ratio = 64; > > > i2s->pinctrl = devm_pinctrl_get(&pdev->dev); > > > - if (IS_ERR(i2s->pinctrl)) > > > + if (IS_ERR(i2s->pinctrl)) { > > > dev_err(&pdev->dev, "failed to find i2s pinctrl\n"); > > > + ret = PTR_ERR(i2s->pinctrl); > > > + goto err_clk; > > > + } > > > > This would break audio for HDMI audio, which uses an I2S interface > > to feed the HDMI controller and does not need or have pinctrl, but > > here you make pinctrl a requirement. > > > > See https://lore.kernel.org/alsa-devel/20220621185747.2782-1-wens@kernel.org/ > > for my fix, which is merged for 5.20. > > For what it's worth, I've tested the fix and the board boots just fine. Check if your HDMI audio card probed correctly or not? ChenYu > Thanks, > Alex > > > > > Maybe your patch (which Mark already applied) and commit a5450aba737d > > ("ASoC: rockchip: i2s: switch BCLK to GPIO") should just be reverted from > > the for-5.19? > > > > Mark? > > > > > > Regards > > ChenYu > > > > > > > > i2s->bclk_on = pinctrl_lookup_state(i2s->pinctrl, > > > "bclk_on"); > > > -- > > > 2.37.0 > > > > > > > > > _______________________________________________ > > > Linux-rockchip mailing list > > > Linux-rockchip@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hi, On Tue, Jul 12, 2022 at 05:25:43PM +0800, Chen-Yu Tsai wrote: > On Tue, Jul 12, 2022 at 4:53 PM Alexandru Elisei > <alexandru.elisei@arm.com> wrote: > > > > Hi ChenYu, > > > > On Tue, Jul 12, 2022 at 02:17:32PM +0800, Chen-Yu Tsai wrote: > > > On Mon, Jul 11, 2022 at 9:06 PM Alexandru Elisei > > > <alexandru.elisei@arm.com> wrote: > > > > > > > > Commit a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") switched > > > > BCLK to GPIO functions when probing the i2s bus interface, but missed > > > > adding a check for when devm_pinctrl_get() returns an error. This can lead > > > > to the following NULL pointer dereference on a rockpro64-v2 if there are no > > > > "pinctrl" properties in the i2s device tree node: > > > > > > > > [ 0.658381] rockchip-i2s ff880000.i2s: failed to find i2s default state > > > > [ 0.658993] rockchip-i2s ff880000.i2s: failed to find i2s gpio state > > > > [ 0.660072] rockchip-i2s ff890000.i2s: failed to find i2s default state > > > > [ 0.660670] rockchip-i2s ff890000.i2s: failed to find i2s gpio state > > > > [ 0.661716] rockchip-i2s ff8a0000.i2s: failed to find i2s pinctrl > > > > [ 0.662276] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000005 > > > > [ 0.663061] Mem abort info: > > > > [ 0.663317] ESR = 0x0000000096000004 > > > > [ 0.663658] EC = 0x25: DABT (current EL), IL = 32 bits > > > > [ 0.664136] SET = 0, FnV = 0 > > > > [ 0.664171] mmc2: SDHCI controller on fe330000.mmc [fe330000.mmc] using ADMA > > > > [ 0.664409] EA = 0, S1PTW = 0 > > > > [ 0.664415] FSC = 0x04: level 0 translation fault > > > > [ 0.664421] Data abort info: > > > > [ 0.666050] ISV = 0, ISS = 0x00000004 > > > > [ 0.666399] CM = 0, WnR = 0 > > > > [ 0.666671] [0000000000000005] user address but active_mm is swapper > > > > [ 0.667240] Internal error: Oops: 96000004 [#1] PREEMPT SMP > > > > [ 0.667742] Modules linked in: > > > > [ 0.668028] CPU: 5 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc6 #300 > > > > [ 0.668608] Hardware name: Pine64 RockPro64 v2.0 (DT) > > > > [ 0.669062] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > > > [ 0.669689] pc : pinctrl_lookup_state+0x20/0xc0 > > > > [ 0.670110] lr : rockchip_i2s_probe+0x1a8/0x54c > > > > [ 0.670527] sp : ffff80000a17bb30 > > > > [ 0.670829] x29: ffff80000a17bb30 x28: 0000000000000000 x27: ffff8000097c04c8 > > > > [ 0.671480] x26: ffff800009871060 x25: ffff800009871078 x24: ffff000001c11368 > > > > [ 0.672129] x23: ffff8000092dc850 x22: ffffffffffffffed x21: ffff8000096f7e98 > > > > [ 0.672776] x20: ffffffffffffffed x19: ffff000001d92480 x18: ffffffffffffffff > > > > [ 0.673423] x17: 000000040044ffff x16: ffff0000f77db2d0 x15: 0764076e07690766 > > > > [ 0.674070] x14: 0720076f07740720 x13: ffff800009e129f0 x12: 000000000000038d > > > > [ 0.674717] x11: 000000000000012f x10: ffff800009e6a9f0 x9 : ffff800009e129f0 > > > > [ 0.675364] x8 : 00000000ffffefff x7 : ffff800009e6a9f0 x6 : 80000000fffff000 > > > > [ 0.676011] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 > > > > [ 0.676657] x2 : 0000000000000000 x1 : ffff8000096f7e98 x0 : ffffffffffffffed > > > > [ 0.677304] Call trace: > > > > [ 0.677531] pinctrl_lookup_state+0x20/0xc0 > > > > [ 0.677914] rockchip_i2s_probe+0x1a8/0x54c > > > > [ 0.678297] platform_probe+0x68/0xc0 > > > > [ 0.678638] really_probe.part.0+0x9c/0x2ac > > > > [ 0.679027] __driver_probe_device+0x98/0x144 > > > > [ 0.679429] driver_probe_device+0xac/0x140 > > > > [ 0.679814] __driver_attach+0xf8/0x184 > > > > [ 0.680169] bus_for_each_dev+0x70/0xd0 > > > > [ 0.680524] driver_attach+0x24/0x30 > > > > [ 0.680856] bus_add_driver+0x150/0x200 > > > > [ 0.681210] driver_register+0x78/0x130 > > > > [ 0.681560] __platform_driver_register+0x28/0x34 > > > > [ 0.681988] rockchip_i2s_driver_init+0x1c/0x28 > > > > [ 0.682407] do_one_initcall+0x50/0x1c0 > > > > [ 0.682760] kernel_init_freeable+0x204/0x288 > > > > [ 0.683160] kernel_init+0x28/0x13c > > > > [ 0.683482] ret_from_fork+0x10/0x20 > > > > [ 0.683816] Code: aa0003f4 a9025bf5 aa0003f6 aa0103f5 (f8418e93) > > > > [ 0.684365] ---[ end trace 0000000000000000 ]--- > > > > [ 0.684813] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b > > > > [ 0.685500] SMP: stopping secondary CPUs > > > > [ 0.685995] Kernel Offset: disabled > > > > [ 0.686310] CPU features: 0x800,00105811,00001086 > > > > [ 0.686736] Memory Limit: none > > > > [ 0.687021] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]--- > > > > > > > > Check that i2s->pinctrl is valid before attempting to search for the > > > > bclk_on and bclk_off pinctrl states. > > > > > > > > Fixes: a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") > > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > > > > --- > > > > > > > > Full log at [1], config at [2] (pastebins expire after 6 months). > > > > > > > > I'm not familiar with this part of the kernel, I did my best to come up > > > > with an explanation and a fix for the panic. > > > > > > > > Read Documentation/devicetree/bindings/sound/rockchip-i2s.yaml, which has the > > > > definition for the i2s nodes with the same compatible string as the i2s@ff8a0000 > > > > that is causing the panic (which is, "rockchip,rk3399-i2s" > > > > "rockchip,rk3066-i2s"). There's no mention there of a "pinctrl" property, maybe > > > > I'm reading the docs wrong, or maybe the board devicetree also needs fixing. > > > > > > > > [1] https://pastebin.com/vuRVDsKk > > > > [2] https://pastebin.com/3yDMF7YE > > > > > > > > sound/soc/rockchip/rockchip_i2s.c | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c > > > > index 99a128a666fb..c9fedf6eb2e6 100644 > > > > --- a/sound/soc/rockchip/rockchip_i2s.c > > > > +++ b/sound/soc/rockchip/rockchip_i2s.c > > > > @@ -808,8 +808,11 @@ static int rockchip_i2s_probe(struct platform_device *pdev) > > > > > > > > i2s->bclk_ratio = 64; > > > > i2s->pinctrl = devm_pinctrl_get(&pdev->dev); > > > > - if (IS_ERR(i2s->pinctrl)) > > > > + if (IS_ERR(i2s->pinctrl)) { > > > > dev_err(&pdev->dev, "failed to find i2s pinctrl\n"); > > > > + ret = PTR_ERR(i2s->pinctrl); > > > > + goto err_clk; > > > > + } > > > > > > This would break audio for HDMI audio, which uses an I2S interface > > > to feed the HDMI controller and does not need or have pinctrl, but > > > here you make pinctrl a requirement. > > > > > > See https://lore.kernel.org/alsa-devel/20220621185747.2782-1-wens@kernel.org/ > > > for my fix, which is merged for 5.20. > > > > For what it's worth, I've tested the fix and the board boots just fine. > > Check if your HDMI audio card probed correctly or not? I use my board headless, I didn't even have the HDMI sound card compiled. Compiled the hdmi sound driver now (CONFIG_SND_SOC=y, CONFIG_SND_SIMPLE_CARD=y), and when I do aplay -L I don't see a hdmi-sound device. Sprinkled some printk's in asoc_simple_probe(), simple_parse_of() returns -517. Same for v5.19-rc5. Any suggestions? Thanks, Alex > > ChenYu > > > Thanks, > > Alex > > > > > > > > Maybe your patch (which Mark already applied) and commit a5450aba737d > > > ("ASoC: rockchip: i2s: switch BCLK to GPIO") should just be reverted from > > > the for-5.19? > > > > > > Mark? > > > > > > > > > Regards > > > ChenYu > > > > > > > > > > > i2s->bclk_on = pinctrl_lookup_state(i2s->pinctrl, > > > > "bclk_on"); > > > > -- > > > > 2.37.0 > > > > > > > > > > > > _______________________________________________ > > > > Linux-rockchip mailing list > > > > Linux-rockchip@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-rockchip
On Tue, Jul 12, 2022 at 12:02:02PM +0100, Alexandru Elisei wrote: > On Tue, Jul 12, 2022 at 05:25:43PM +0800, Chen-Yu Tsai wrote: > Any suggestions? I think Chen-Yu's suggestion of reverting both patches for 5.19 is probably the most sensible one at this point.
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 99a128a666fb..c9fedf6eb2e6 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -808,8 +808,11 @@ static int rockchip_i2s_probe(struct platform_device *pdev) i2s->bclk_ratio = 64; i2s->pinctrl = devm_pinctrl_get(&pdev->dev); - if (IS_ERR(i2s->pinctrl)) + if (IS_ERR(i2s->pinctrl)) { dev_err(&pdev->dev, "failed to find i2s pinctrl\n"); + ret = PTR_ERR(i2s->pinctrl); + goto err_clk; + } i2s->bclk_on = pinctrl_lookup_state(i2s->pinctrl, "bclk_on");
Commit a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") switched BCLK to GPIO functions when probing the i2s bus interface, but missed adding a check for when devm_pinctrl_get() returns an error. This can lead to the following NULL pointer dereference on a rockpro64-v2 if there are no "pinctrl" properties in the i2s device tree node: [ 0.658381] rockchip-i2s ff880000.i2s: failed to find i2s default state [ 0.658993] rockchip-i2s ff880000.i2s: failed to find i2s gpio state [ 0.660072] rockchip-i2s ff890000.i2s: failed to find i2s default state [ 0.660670] rockchip-i2s ff890000.i2s: failed to find i2s gpio state [ 0.661716] rockchip-i2s ff8a0000.i2s: failed to find i2s pinctrl [ 0.662276] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000005 [ 0.663061] Mem abort info: [ 0.663317] ESR = 0x0000000096000004 [ 0.663658] EC = 0x25: DABT (current EL), IL = 32 bits [ 0.664136] SET = 0, FnV = 0 [ 0.664171] mmc2: SDHCI controller on fe330000.mmc [fe330000.mmc] using ADMA [ 0.664409] EA = 0, S1PTW = 0 [ 0.664415] FSC = 0x04: level 0 translation fault [ 0.664421] Data abort info: [ 0.666050] ISV = 0, ISS = 0x00000004 [ 0.666399] CM = 0, WnR = 0 [ 0.666671] [0000000000000005] user address but active_mm is swapper [ 0.667240] Internal error: Oops: 96000004 [#1] PREEMPT SMP [ 0.667742] Modules linked in: [ 0.668028] CPU: 5 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc6 #300 [ 0.668608] Hardware name: Pine64 RockPro64 v2.0 (DT) [ 0.669062] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 0.669689] pc : pinctrl_lookup_state+0x20/0xc0 [ 0.670110] lr : rockchip_i2s_probe+0x1a8/0x54c [ 0.670527] sp : ffff80000a17bb30 [ 0.670829] x29: ffff80000a17bb30 x28: 0000000000000000 x27: ffff8000097c04c8 [ 0.671480] x26: ffff800009871060 x25: ffff800009871078 x24: ffff000001c11368 [ 0.672129] x23: ffff8000092dc850 x22: ffffffffffffffed x21: ffff8000096f7e98 [ 0.672776] x20: ffffffffffffffed x19: ffff000001d92480 x18: ffffffffffffffff [ 0.673423] x17: 000000040044ffff x16: ffff0000f77db2d0 x15: 0764076e07690766 [ 0.674070] x14: 0720076f07740720 x13: ffff800009e129f0 x12: 000000000000038d [ 0.674717] x11: 000000000000012f x10: ffff800009e6a9f0 x9 : ffff800009e129f0 [ 0.675364] x8 : 00000000ffffefff x7 : ffff800009e6a9f0 x6 : 80000000fffff000 [ 0.676011] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 [ 0.676657] x2 : 0000000000000000 x1 : ffff8000096f7e98 x0 : ffffffffffffffed [ 0.677304] Call trace: [ 0.677531] pinctrl_lookup_state+0x20/0xc0 [ 0.677914] rockchip_i2s_probe+0x1a8/0x54c [ 0.678297] platform_probe+0x68/0xc0 [ 0.678638] really_probe.part.0+0x9c/0x2ac [ 0.679027] __driver_probe_device+0x98/0x144 [ 0.679429] driver_probe_device+0xac/0x140 [ 0.679814] __driver_attach+0xf8/0x184 [ 0.680169] bus_for_each_dev+0x70/0xd0 [ 0.680524] driver_attach+0x24/0x30 [ 0.680856] bus_add_driver+0x150/0x200 [ 0.681210] driver_register+0x78/0x130 [ 0.681560] __platform_driver_register+0x28/0x34 [ 0.681988] rockchip_i2s_driver_init+0x1c/0x28 [ 0.682407] do_one_initcall+0x50/0x1c0 [ 0.682760] kernel_init_freeable+0x204/0x288 [ 0.683160] kernel_init+0x28/0x13c [ 0.683482] ret_from_fork+0x10/0x20 [ 0.683816] Code: aa0003f4 a9025bf5 aa0003f6 aa0103f5 (f8418e93) [ 0.684365] ---[ end trace 0000000000000000 ]--- [ 0.684813] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b [ 0.685500] SMP: stopping secondary CPUs [ 0.685995] Kernel Offset: disabled [ 0.686310] CPU features: 0x800,00105811,00001086 [ 0.686736] Memory Limit: none [ 0.687021] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]--- Check that i2s->pinctrl is valid before attempting to search for the bclk_on and bclk_off pinctrl states. Fixes: a5450aba737d ("ASoC: rockchip: i2s: switch BCLK to GPIO") Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- Full log at [1], config at [2] (pastebins expire after 6 months). I'm not familiar with this part of the kernel, I did my best to come up with an explanation and a fix for the panic. Read Documentation/devicetree/bindings/sound/rockchip-i2s.yaml, which has the definition for the i2s nodes with the same compatible string as the i2s@ff8a0000 that is causing the panic (which is, "rockchip,rk3399-i2s" "rockchip,rk3066-i2s"). There's no mention there of a "pinctrl" property, maybe I'm reading the docs wrong, or maybe the board devicetree also needs fixing. [1] https://pastebin.com/vuRVDsKk [2] https://pastebin.com/3yDMF7YE sound/soc/rockchip/rockchip_i2s.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)