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

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

[Alexandru Elisei]

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;
+	}
 
 	i2s->bclk_on = pinctrl_lookup_state(i2s->pinctrl,
 				   "bclk_on");
-- 
2.37.0

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

[Alexandru Elisei]

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;
+	}
 
 	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

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

[Alexandru Elisei]

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;
+	}
 
 	i2s->bclk_on = pinctrl_lookup_state(i2s->pinctrl,
 				   "bclk_on");
-- 
2.37.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1120 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1120 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1996 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1996 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1996 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1996 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 164 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 164 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 164 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 164 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Chen-Yu Tsai]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Chen-Yu Tsai]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Chen-Yu Tsai]

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Chen-Yu Tsai]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Chen-Yu Tsai]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Chen-Yu Tsai]

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Chen-Yu Tsai]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Chen-Yu Tsai]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Alexandru Elisei]

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

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 264 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 264 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 264 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] ASoC: rockchip: i2s: Fix NULL pointer dereference when pinctrl is not found

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 264 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel