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 <[email protected]>
---
.../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";
+ *
+ * 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 {
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";
--
2.42.0
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 <[email protected]>
> ---
> .../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 <[email protected]> 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 <[email protected]> wrote:
> On Sunday, March 24th, 2024 at 16:15, Krzysztof Kozlowski [email protected] 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 <[email protected]> wrote:
>
>> On Sunday, March 24th, 2024 at 16:15, Krzysztof Kozlowski [email protected] 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) <[email protected]> wrote:
>
>
> On 24.03.24 12:43, Pratham Patel wrote:
>
> > On Sunday, March 24th, 2024 at 16:51, Pratham Patel [email protected] wrote:
> >
> > > On Sunday, March 24th, 2024 at 16:15, Krzysztof Kozlowski [email protected] 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 <[email protected]> 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 <[email protected]> wrote:
>
>
> On 24/03/2024 12:21, Pratham Patel wrote:
>
> > On Sunday, March 24th, 2024 at 16:15, Krzysztof Kozlowski [email protected] 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