diff mbox series

ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

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

Commit Message

Alexandru Elisei July 11, 2022, 1:05 p.m. UTC
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(-)

Comments

Alexandru Elisei July 11, 2022, 1:40 p.m. UTC | #1
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
Alexandru Elisei July 11, 2022, 3:17 p.m. UTC | #2
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
Alexandru Elisei July 12, 2022, 8:54 a.m. UTC | #3
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
Alexandru Elisei July 12, 2022, 11:02 a.m. UTC | #4
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
diff mbox series

Patch

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");