2022-10-20 02:24:25

by Bard Liao

[permalink] [raw]
Subject: [PATCH] soundwire: intel: Initialize clock stop timeout

From: Sjoerd Simons <[email protected]>

The bus->clk_stop_timeout member is only initialized to a non-zero value
during the codec driver probe. This can lead to corner cases where this
value remains pegged at zero when the bus suspends, which results in an
endless loop in sdw_bus_wait_for_clk_prep_deprep().

Corner cases include configurations with no codecs described in the
firmware, or delays in probing codec drivers.

Initializing the default timeout to the smallest non-zero value avoid this
problem and allows for the existing logic to be preserved: the
bus->clk_stop_timeout is set as the maximum required by all codecs
connected on the bus.

Fixes: 1f2dcf3a154ac ("soundwire: intel: set dev_num_ida_min")
Signed-off-by: Sjoerd Simons <[email protected]>
Reviewed-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Chao Song <[email protected]>
Signed-off-by: Bard Liao <[email protected]>
---
drivers/soundwire/intel.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 25ec9c272239..78d35bb4852c 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1311,6 +1311,7 @@ static int intel_link_probe(struct auxiliary_device *auxdev,

bus->link_id = auxdev->id;
bus->dev_num_ida_min = INTEL_DEV_NUM_IDA_MIN;
+ bus->clk_stop_timeout = 1;

sdw_cdns_probe(cdns);

--
2.25.1


2022-10-21 17:21:16

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [PATCH] soundwire: intel: Initialize clock stop timeout



On 10/19/22 20:56, Bard Liao wrote:
> From: Sjoerd Simons <[email protected]>
>
> The bus->clk_stop_timeout member is only initialized to a non-zero value
> during the codec driver probe. This can lead to corner cases where this
> value remains pegged at zero when the bus suspends, which results in an
> endless loop in sdw_bus_wait_for_clk_prep_deprep().
>
> Corner cases include configurations with no codecs described in the
> firmware, or delays in probing codec drivers.
>
> Initializing the default timeout to the smallest non-zero value avoid this
> problem and allows for the existing logic to be preserved: the
> bus->clk_stop_timeout is set as the maximum required by all codecs
> connected on the bus.
>
> Fixes: 1f2dcf3a154ac ("soundwire: intel: set dev_num_ida_min")
> Signed-off-by: Sjoerd Simons <[email protected]>
> Reviewed-by: Pierre-Louis Bossart <[email protected]>
> Reviewed-by: Chao Song <[email protected]>
> Signed-off-by: Bard Liao <[email protected]>

this patch should be sent to GregKH/Linus as a 6.1-rcx fix, it does seem
to make the life of Arch/Debian users less miserable - for some reason
very large delays on driver probe seem to trigger this corner case and
make things even worse.

see https://github.com/thesofproject/linux/issues/3777 for details.

Thanks Vinod.

> ---
> drivers/soundwire/intel.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> index 25ec9c272239..78d35bb4852c 100644
> --- a/drivers/soundwire/intel.c
> +++ b/drivers/soundwire/intel.c
> @@ -1311,6 +1311,7 @@ static int intel_link_probe(struct auxiliary_device *auxdev,
>
> bus->link_id = auxdev->id;
> bus->dev_num_ida_min = INTEL_DEV_NUM_IDA_MIN;
> + bus->clk_stop_timeout = 1;
>
> sdw_cdns_probe(cdns);
>

2022-10-28 12:22:43

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH] soundwire: intel: Initialize clock stop timeout

On 20-10-22, 09:56, Bard Liao wrote:
> From: Sjoerd Simons <[email protected]>
>
> The bus->clk_stop_timeout member is only initialized to a non-zero value
> during the codec driver probe. This can lead to corner cases where this
> value remains pegged at zero when the bus suspends, which results in an
> endless loop in sdw_bus_wait_for_clk_prep_deprep().
>
> Corner cases include configurations with no codecs described in the
> firmware, or delays in probing codec drivers.
>
> Initializing the default timeout to the smallest non-zero value avoid this
> problem and allows for the existing logic to be preserved: the
> bus->clk_stop_timeout is set as the maximum required by all codecs
> connected on the bus.

Applied to fixes, thanks

--
~Vinod

2022-11-09 16:39:34

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [PATCH] soundwire: intel: Initialize clock stop timeout



On 10/28/22 06:28, Vinod Koul wrote:
> On 20-10-22, 09:56, Bard Liao wrote:
>> From: Sjoerd Simons <[email protected]>
>>
>> The bus->clk_stop_timeout member is only initialized to a non-zero value
>> during the codec driver probe. This can lead to corner cases where this
>> value remains pegged at zero when the bus suspends, which results in an
>> endless loop in sdw_bus_wait_for_clk_prep_deprep().
>>
>> Corner cases include configurations with no codecs described in the
>> firmware, or delays in probing codec drivers.
>>
>> Initializing the default timeout to the smallest non-zero value avoid this
>> problem and allows for the existing logic to be preserved: the
>> bus->clk_stop_timeout is set as the maximum required by all codecs
>> connected on the bus.
>
> Applied to fixes, thanks

Thanks Vinod, was this sent to Greg/Linus? the last pull request I see
was for 6.1-rc1.
Arch Linux cherry-picked this patch but other distros did not, so quite
a few users are left with no audio card.

2022-11-11 04:43:04

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH] soundwire: intel: Initialize clock stop timeout

On 09-11-22, 10:05, Pierre-Louis Bossart wrote:
>
>
> On 10/28/22 06:28, Vinod Koul wrote:
> > On 20-10-22, 09:56, Bard Liao wrote:
> >> From: Sjoerd Simons <[email protected]>
> >>
> >> The bus->clk_stop_timeout member is only initialized to a non-zero value
> >> during the codec driver probe. This can lead to corner cases where this
> >> value remains pegged at zero when the bus suspends, which results in an
> >> endless loop in sdw_bus_wait_for_clk_prep_deprep().
> >>
> >> Corner cases include configurations with no codecs described in the
> >> firmware, or delays in probing codec drivers.
> >>
> >> Initializing the default timeout to the smallest non-zero value avoid this
> >> problem and allows for the existing logic to be preserved: the
> >> bus->clk_stop_timeout is set as the maximum required by all codecs
> >> connected on the bus.
> >
> > Applied to fixes, thanks
>
> Thanks Vinod, was this sent to Greg/Linus? the last pull request I see
> was for 6.1-rc1.
> Arch Linux cherry-picked this patch but other distros did not, so quite
> a few users are left with no audio card.

https://git.kernel.org/torvalds/c/f014699cca9a9a28fbdc06a9225b54562154fc20


--
~Vinod