| Message ID | 20240324062816.145858-1-prathampatel@thefossguy.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] arm64: dts: rockchip: disable analog audio for rock-5b | expand |
On 24/03/2024 07:28, Pratham Patel wrote: > The addition of `of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing` Please refer to commits using commit sha () syntax, as mentioned in submitting patches. > has surfaced an issue with the analog audio property in the devicetree > for the rock-5b. Booting kernels v6.7.9+ and v6.8.0+ would cause the > following call trace: > > [ 21.595068] Call trace: > [ 21.595288] smp_call_function_many_cond+0x174/0x5f8 > [ 21.595728] on_each_cpu_cond_mask+0x2c/0x40 > [ 21.596109] cpuidle_register_driver+0x294/0x318 > [ 21.596524] cpuidle_register+0x24/0x100 > [ 21.596875] psci_cpuidle_probe+0x2e4/0x490 > [ 21.597247] platform_probe+0x70/0xd0 > [ 21.597575] really_probe+0x18c/0x3d8 > [ 21.597905] __driver_probe_device+0x84/0x180 > [ 21.598294] driver_probe_device+0x44/0x120 > [ 21.598669] __device_attach_driver+0xc4/0x168 > [ 21.599063] bus_for_each_drv+0x8c/0xf0 > [ 21.599408] __device_attach+0xa4/0x1c0 > [ 21.599748] device_initial_probe+0x1c/0x30 > [ 21.600118] bus_probe_device+0xb4/0xc0 > [ 21.600462] device_add+0x68c/0x888 > [ 21.600775 > ] platform_device_add+0x19c/0x270 > [ 21.601154] platform_device_register_full+0xdc/0x178 > [ 21.601602] psci_idle_init+0xa0/0xc8 > [ 21.601934] do_one_initcall+0x60/0x290 > [ 21.602275] kernel_init_freeable+0x20c/0x3e0 > [ 21.602664] kernel_init+0x2c/0x1f8 > [ 21.602979] ret_from_fork+0x10/0x20 > > This is a temporary workaround to at least have the SBC boot. There are > a few more SBCs that _might_ have this issue. I suspect that the > rock-5a and nanopc-t6 might also have this issue but I do not own either > board to verify this claim, yet. > > Closes: https://lore.kernel.org/regressions/28S1EMw5YOnQIBpQ8_qaZZ6c9Go-j6-lLuWWbRpe6-MtRUd7Ay-uXq8JHbVVtJv3LzpxjI8jYg7ukNntbN22PVV-hOWbuTY8FNWgvM4zSwI=@thefossguy.com/T/#m69eedea6fbcb0591d54a9ccd478c2782ef045547 > > Signed-off-by: Pratham Patel <prathampatel@thefossguy.com> > --- > .../boot/dts/rockchip/rk3588-rock-5b.dts | 110 +++++++++--------- > 1 file changed, 57 insertions(+), 53 deletions(-) > > diff --git a/arch/arm64/boot/d > ts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts > index 1fe8b2a0e..6d3b9f52c 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts > @@ -20,22 +20,24 @@ chosen { > stdout-path = "serial2:1500000n8"; > }; > > - analog-sound { > - compatible = "audio-graph-card"; > - label = "rk3588-es8316"; > - > - widgets = "Microphone", "Mic Jack", > - "Headphone", "Headphones"; > - > - routing = "MIC2", "Mic Jack", > - "Headphones", "HPOL", > - "Headphones", "HPOR"; > - > - dais = <&i2s0_8ch_p0>; > - hp-det-gpio = <&gpio1 RK_PD5 GPIO_ACTIVE_HIGH>; > - pinctrl-names = "default"; > - pinctrl-0 = <&hp_detect>; > - }; > + /* > + *analog-sound { > + * compatible = "audio-graph-card"; > + * label = "rk3588-es8316"; Do not comment out code. Instead disable the nodes and provide appropriate comment describing reason. Anyway, this does not look like correct solution. DTS is independent of OS, so bug in fwlink does not matter for DTS. Either DTS is a correct hardware representation or not. Best regards, Krzysztof
On Sunday, March 24th, 2024 at 16:15, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > > On 24/03/2024 07:28, Pratham Patel wrote: > > > The addition of `of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing` > > > Please refer to commits using commit sha () syntax, as mentioned in > submitting patches. Noticed that in the wiki but didn't do that since the commit hash for the commit was different in each branch (of the stable tree). Maybe I should have copied the SHA from Linus' tree. I will do that. > > + /* > > + *analog-sound { > > + * compatible = "audio-graph-card"; > > + * label = "rk3588-es8316"; > > > Do not comment out code. Instead disable the nodes and provide > appropriate comment describing reason. I tried changing the status from okay to disabled. That didn't work. The SBC still locked up during boot. > Anyway, this does not look like correct solution. DTS is independent of > OS, so bug in fwlink does not matter for DTS. Either DTS is a correct > hardware representation or not. I agree, it's not the correct solution. It is a temporary workaround for the regression caused. I will send more patches once I receive a few more RK3588-based SBCs and investigate. -- Pratham Patel
On Sunday, March 24th, 2024 at 16:51, Pratham Patel <prathampatel@thefossguy.com> wrote: > On Sunday, March 24th, 2024 at 16:15, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote: > > > > + /* > > > + *analog-sound { > > > + * compatible = "audio-graph-card"; > > > + * label = "rk3588-es8316"; > > > > Do not comment out code. Instead disable the nodes and provide > > appropriate comment describing reason. > > I tried changing the status from okay to disabled. That didn't work. The SBC > still locked up during boot. I think setting the status to fail should do the trick, instead of setting it to disabled. Will try that and be back with a v2. -- Pratham Patel
On 24.03.24 12:43, Pratham Patel wrote: > On Sunday, March 24th, 2024 at 16:51, Pratham Patel <prathampatel@thefossguy.com> wrote: > >> On Sunday, March 24th, 2024 at 16:15, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote: >> >>>> + /* >>>> + *analog-sound { >>>> + * compatible = "audio-graph-card"; >>>> + * label = "rk3588-es8316"; >>> >>> Do not comment out code. Instead disable the nodes and provide >>> appropriate comment describing reason. >> >> I tried changing the status from okay to disabled. That didn't work. The SBC >> still locked up during boot. > > I think setting the status to fail should do the trick, instead of setting it to disabled. > Will try that and be back with a v2. Please CC the author of the change that broke things when submitting v2, which you afaics failed to do in this thread. Ciao, Thorsten
On Sunday, March 24th, 2024 at 17:44, Linux regression tracking (Thorsten Leemhuis) <regressions@leemhuis.info> wrote: > > > On 24.03.24 12:43, Pratham Patel wrote: > > > On Sunday, March 24th, 2024 at 16:51, Pratham Patel prathampatel@thefossguy.com wrote: > > > > > On Sunday, March 24th, 2024 at 16:15, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote: > > > > > > > > + /* > > > > > + *analog-sound { > > > > > + * compatible = "audio-graph-card"; > > > > > + * label = "rk3588-es8316"; > > > > > > > > Do not comment out code. Instead disable the nodes and provide > > > > appropriate comment describing reason. > > > > > > I tried changing the status from okay to disabled. That didn't work. The SBC > > > still locked up during boot. > > > > I think setting the status to fail should do the trick, instead of setting it to disabled. > > Will try that and be back with a v2. > > > Please CC the author of the change that broke things when submitting v2, > which you afaics failed to do in this thread. > > Ciao, Thorsten Ack, will do. -- Pratham Patel
On 24/03/2024 12:21, Pratham Patel wrote: > On Sunday, March 24th, 2024 at 16:15, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >> >> >> On 24/03/2024 07:28, Pratham Patel wrote: >> >>> The addition of `of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing` >> >> >> Please refer to commits using commit sha () syntax, as mentioned in >> submitting patches. > > Noticed that in the wiki but didn't do that since the commit hash for the commit > was different in each branch (of the stable tree). Maybe I should have copied the SHA > from Linus' tree. I will do that. There is only one tree: master/mainline tree. Commits in stable do not matter outside of stable. Best regards, Krzysztof
On Monday, March 25th, 2024 at 13:18, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > > On 24/03/2024 12:21, Pratham Patel wrote: > > > On Sunday, March 24th, 2024 at 16:15, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote: > > > > > On 24/03/2024 07:28, Pratham Patel wrote: > > > > > > > The addition of `of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing` > > > > > > Please refer to commits using commit sha () syntax, as mentioned in > > > submitting patches. > > > > Noticed that in the wiki but didn't do that since the commit hash for the commit > > was different in each branch (of the stable tree). Maybe I should have copied the SHA > > from Linus' tree. I will do that. > > > There is only one tree: master/mainline tree. Commits in stable do not > matter outside of stable. Ack, thank you. -- Pratham Patel
diff --git a/arch/arm64/boot/d ts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts index 1fe8b2a0e..6d3b9f52c 100644 --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts @@ -20,22 +20,24 @@ chosen { stdout-path = "serial2:1500000n8"; }; - analog-sound { - compatible = "audio-graph-card"; - label = "rk3588-es8316"; - - widgets = "Microphone", "Mic Jack", - "Headphone", "Headphones"; - - routing = "MIC2", "Mic Jack", - "Headphones", "HPOL", - "Headphones", "HPOR"; - - dais = <&i2s0_8ch_p0>; - hp-det-gpio = <&gpio1 RK_PD5 GPIO_ACTIVE_HIGH>; - pinctrl-names = "default"; - pinctrl-0 = <&hp_detect>; - }; + /* + *analog-sound { + * compatible = "audio-graph-card"; + * label = "rk3588-es8316"; + * + * widgets = "Microphone", "Mic Jack", + * "Headphone", "Headphones"; + * + * routing = "MIC2", "Mic Jack", + * "Headphones", "HPOL", + * "Headphones", "HPOR"; + * + * dais = <&i2s0_8ch_p0>; + * hp-det-gpio = <&gpio1 RK_PD5 GPIO_ACTIVE_HIGH>; + * pinctrl-names = "default"; + * pinctrl-0 = <&hp_detect>; + *}; + */ leds {
The addition of `of: property: fw_devlink: Fix stupid bug in remote-endpoint parsing` has surfaced an issue with the analog audio property in the devicetree for the rock-5b. Booting kernels v6.7.9+ and v6.8.0+ would cause the following call trace: [ 21.595068] Call trace: [ 21.595288] smp_call_function_many_cond+0x174/0x5f8 [ 21.595728] on_each_cpu_cond_mask+0x2c/0x40 [ 21.596109] cpuidle_register_driver+0x294/0x318 [ 21.596524] cpuidle_register+0x24/0x100 [ 21.596875] psci_cpuidle_probe+0x2e4/0x490 [ 21.597247] platform_probe+0x70/0xd0 [ 21.597575] really_probe+0x18c/0x3d8 [ 21.597905] __driver_probe_device+0x84/0x180 [ 21.598294] driver_probe_device+0x44/0x120 [ 21.598669] __device_attach_driver+0xc4/0x168 [ 21.599063] bus_for_each_drv+0x8c/0xf0 [ 21.599408] __device_attach+0xa4/0x1c0 [ 21.599748] device_initial_probe+0x1c/0x30 [ 21.600118] bus_probe_device+0xb4/0xc0 [ 21.600462] device_add+0x68c/0x888 [ 21.600775 ] platform_device_add+0x19c/0x270 [ 21.601154] platform_device_register_full+0xdc/0x178 [ 21.601602] psci_idle_init+0xa0/0xc8 [ 21.601934] do_one_initcall+0x60/0x290 [ 21.602275] kernel_init_freeable+0x20c/0x3e0 [ 21.602664] kernel_init+0x2c/0x1f8 [ 21.602979] ret_from_fork+0x10/0x20 This is a temporary workaround to at least have the SBC boot. There are a few more SBCs that _might_ have this issue. I suspect that the rock-5a and nanopc-t6 might also have this issue but I do not own either board to verify this claim, yet. Closes: https://lore.kernel.org/regressions/28S1EMw5YOnQIBpQ8_qaZZ6c9Go-j6-lLuWWbRpe6-MtRUd7Ay-uXq8JHbVVtJv3LzpxjI8jYg7ukNntbN22PVV-hOWbuTY8FNWgvM4zSwI=@thefossguy.com/T/#m69eedea6fbcb0591d54a9ccd478c2782ef045547 Signed-off-by: Pratham Patel <prathampatel@thefossguy.com> --- .../boot/dts/rockchip/rk3588-rock-5b.dts | 110 +++++++++--------- 1 file changed, 57 insertions(+), 53 deletions(-) compatible = "gpio-leds"; @@ -236,43 +238,45 @@ hym8563: rtc@51 { }; }; -&i2c7 { - status = "okay"; - - es8316: audio-codec@11 { - compatible = "everest,es8316"; - reg = <0x11>; - clocks = <&cru I2S0_8CH_MCLKOUT>; - clock-names = "mclk"; - assigned-clocks = <&cru I2S0_8CH_MCLKOUT>; - assigned-clock-rates = <12288000>; - #sound-dai-cells = <0>; - - port { - es8316_p0_0: endpoint { - remote-endpoint = <&i2s0_8ch_p0_0>; - }; - }; - }; -}; - -&i2s0_8ch { - pinctrl-names = "default"; - pinctrl-0 = <&i2s0_lrck - &i2s0_mclk - &i2s0_sclk - &i2s0_sdi0 - &i2s0_sdo0>; - status = "okay"; - - i2s0_8ch_p0: port { - i2s0_8ch_p0_0: endpoint { - dai-format = "i2s"; - mclk-fs = <256>; - remote-endpoint = <&es8316_p0_0>; - }; - }; -}; +/* + *&i2c7 { + * status = "okay"; + * + * es8316: audio-codec@11 { + * compatible = "everest,es8316"; + * reg = <0x11>; + * clocks = <&cru I2S0_8CH_MCLKOUT>; + * clock-names = "mclk"; + * assigned-clocks = <&cru I2S0_8CH_MCLKOUT>; + * assigned-clock-rates = <12288000>; + * #sound-dai-cells = <0>; + * + * port { + * es8316_p0_0: endpoint { + * remote-endpoint = <&i2s0_8ch_p0_0>; + * }; + * }; + * }; + *}; + * + *&i2s0_8ch { + * pinctrl-names = "default"; + * pinctrl-0 = <&i2s0_lrck + * &i2s0_mclk + * &i2s0_sclk + * &i2s0_sdi0 + * &i2s0_sdo0>; + * status = "okay"; + * + * i2s0_8ch_p0: port { + * i2s0_8ch_p0_0: endpoint { + * dai-format = "i2s"; + * mclk-fs = <256>; + * remote-endpoint = <&es8316_p0_0>; + * }; + * }; + *}; + */ &pcie2x1l0 { pinctrl-names = "default";